Page MenuHomePhabricator

run-tests: enforce the drive letter from `getcwd` to upper case
ClosedPublic

Authored by marmoute on Jul 9 2021, 7:48 AM.

Details

Summary

For some reason os.getcwd() can return either c: or C: depending of which
binary you used on Windows. We normalize this to C: and the like. This fix
test-run-tests.t on windows as the drive letter in "$TESTTMP" was "wrongly"
set to 'c:/' if the test path wasn't explicitly specified.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

marmoute created this revision.Jul 9 2021, 7:48 AM

Does the issue happen with 3.7? I'm wondering if this is the normalization they started to do in os.path.realpath(). See 3dfebba99ef6

The same getcwd is called in both case, so, I don't know. I don't have interactive testing on Windows. Can you check ?

(in the mean time, this change does not seems bad in itself, just hacky)

marmoute planned changes to this revision.Jul 9 2021, 7:16 PM

I eventually got to th ebottom of it. os.getcwd return inconsistent capitalization for the drive letter. So any os.path.join is getting poisoned. Unless the path joined is already absolute and it trumps the os.getcwd one.

So I am adjusting the function to return something proper. Update coming.

marmoute retitled this revision from run-test: adjust the drive letter to upper case for TESTDIR to run-tests: enforce the drive letter from `getcwd` to upper case.
marmoute edited the summary of this revision. (Show Details)
marmoute updated this revision to Diff 29055.
baymax updated this revision to Diff 29076.Jul 9 2021, 9:21 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 29096.Jul 9 2021, 11:41 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

marmoute edited the summary of this revision. (Show Details)Jul 10 2021, 1:44 PM
marmoute updated this revision to Diff 29114.
Alphare accepted this revision.Jul 13 2021, 11:21 AM
This revision is now accepted and ready to land.Jul 13 2021, 11:21 AM
marmoute updated this revision to Diff 29232.Jul 13 2021, 2:28 PM