Page MenuHomePhabricator

[Dexter] Add os.path.normcase(...) transform to test path early.
ClosedPublic

Authored by TWeaver on Apr 22 2020, 6:28 AM.

Details

Summary

When passing a test path, if the path points directly at a file, then normcase would not be called on path.

This would change the expected lower case drive path, on windows, to be uppercase.

This patch simply calls normcase on the test path at the earliest point possible to avoid this issue.

Diff Detail

Event Timeline

TWeaver created this revision.Apr 22 2020, 6:28 AM
TWeaver updated this revision to Diff 259267.Apr 22 2020, 6:32 AM

removed accidental new line deletion

jmorse accepted this revision.Apr 27 2020, 3:35 AM

LGTM -- this caused some kind of test failure due to Dexter not recognising the source files, right?

This revision is now accepted and ready to land.Apr 27 2020, 3:35 AM
djtodoro added inline comments.Apr 27 2020, 4:35 AM
debuginfo-tests/dexter/dex/tools/TestToolBase.py
82

All in one line os.path.normcase(os.path.abspath(options.test_path))) ?

@jmorse

no, this would cause file path comparisons that are case sensitive to potentially fail on drive letter for windows.

on investigation is seems the drive letter strings in the windows debug info is lower case, but in dexter we're storing an uppercase when passing a direct file path.

This did cause issues in the on going conditional break point work but may also cause discrepancies in other tests when running under windows.

@djtodoro

I prefer the current approach for readabilities sake as well as separating the two method calls out.

I feel if someone wished to add more code to the path calculation (what ever future reason, should one exist) then the separation shows the distinct steps that already take place.

Two lines does mean however, should someone wish to move these calls, they'd have to read both lines and move them together.

So it's a toss up of which benefit is preferred. I prefer two lines for readabilities sake.

It hardly matters of course and if you insist then I'm happy to accommodate.

Cheers
Tom W

@TWeaver I don't have strong opinion about that, so I am OK with either way. :)