Page MenuHomePhabricator

[lit] Harmonize test timing data between Unix and Windows
ClosedPublic

Authored by davezarzycki on Mar 17 2021, 2:53 AM.

Details

Summary

The "path" recorded for timing purposes is only used as a key into a dictionary. It is never used as an actual path to a filesystem API, therefore we should use '/' as the canonical separator so that Unix and Windows machines can share timing data. This also ensures that the lit testing works across platforms.

Diff Detail

Event Timeline

davezarzycki created this revision.Mar 17 2021, 2:53 AM
davezarzycki requested review of this revision.Mar 17 2021, 2:53 AM
jhenderson accepted this revision.Mar 17 2021, 3:14 AM
jhenderson added a reviewer: jmorse.
jhenderson added a subscriber: jmorse.

@jmorse, could you try this out to see if it fixes the issues you were seeing before? LGTM, assuming it fixes the Windows issue.

This revision is now accepted and ready to land.Mar 17 2021, 3:14 AM
jmorse accepted this revision.Mar 17 2021, 4:40 AM

Passes on Windows just fine, thanks for the follow up!

This revision was landed with ongoing or failed builds.Mar 17 2021, 4:43 AM
This revision was automatically updated to reflect the committed changes.
yln added inline comments.Mar 17 2021, 11:09 AM
llvm/utils/lit/lit/Test.py
265–266

Could we use getFullName() here? Or if the config-qualified name is undesirable, we could extract this into Test::getName().

def getFullName(self):
    return self.suite.config.name + ' :: ' + '/'.join(self.path_in_suite)
davezarzycki added inline comments.Mar 17 2021, 12:12 PM
llvm/utils/lit/lit/Test.py
265–266

All of the timing data is stored per test suite, so prefixing the suite name in this context doesn't solve anything. As to factoring that out, sure. We can do that.