Page MenuHomePhabricator

run-tests: avoid silently switching the hg executable used
Needs ReviewPublic

Authored by mharbison72 on Aug 18 2021, 5:00 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

I noticed an issue testing the installed pyoxidizer binary using --with-hg
where it would pass more tests than if the tests are run with --pyoxidized.
It turns out that _usecorrectpython() augments PATH to include the python
binary and its Scripts directory on Windows, and I happen to have pip
installed Mercurial in that version of python. Therefore, the which() method
picked up this installed executable, and wrote that to the generated hg script
in the custom bin directory.

There's an issue here when running test-run-tests.t with --local, as noted
in the comments. I suspect that some stuff from the main instance of
run-tests.py is bleeding into the *.t file's instances of run-tests.py
because when I print opts.with_hg at the point where is complains that
"--with-hg must specify an executable hg script", the path is in the form
"C:\\\\Users\\\\Matt\\\\...". (Two of those slashes are escapes the test runner
puts into printed lines.) If I hardcode r"C:\Users\Matt\..." in run-tests.py,
then it works fine.

Diff Detail

Repository
rHG Mercurial
Branch
stable
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

mharbison72 created this revision.Aug 18 2021, 5:00 PM

Not sure that we want to take this as-is (even though it does avoid a bad problem outside of the internal tests). I'm hoping someone has some insight and saves me some time trial and erroring through this.

I seems a bit weird to be that we try to manually put self._bindir and self._hgcommand together while if I am not mistaken, they both originate from the splitting of some absolute path somewhere. So keeping that absolute path around would seems safer/cleaner to me. However I might be missing something.

I seems a bit weird to be that we try to manually put self._bindir and self._hgcommand together while if I am not mistaken, they both originate from the splitting of some absolute path somewhere. So keeping that absolute path around would seems safer/cleaner to me. However I might be missing something.

Yeah, that’s where it comes from and I agree it would be better. I’m trying to avoid adding a new variable holding the full path, because I don’t have a way to test that with the rust and chg variants.

I don’t have a way to test that with the rust and chg variants.

You mean under Windows or in general? Because this also feels wrong to me, we should probably be keeping the full path around.

tests/run-tests.py
3682

/s/used/use/