This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11307

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

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

Details

Reviewers
abhishekaggarwal9520817742
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
mharbison72 added a comment.Aug 18 2021, 5:08 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.

marmoute added a subscriber: marmoute.Aug 18 2021, 6:13 PM

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.

mharbison72 added a comment.Aug 18 2021, 7:29 PM
In D11307#173445, @marmoute wrote:

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.

Alphare added a subscriber: Alphare.Aug 25 2021, 5:46 AM
In D11307#173446, @mharbison72 wrote:

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/

abhishekaggarwal9520817742 requested changes to this revision.Dec 25 2021, 2:19 PM

I need my real identity

This revision now requires changes to proceed.Dec 25 2021, 2:19 PM

Revision Contents
Changeset List

PathPackages
Mtests/run-tests.py (15 lines)

Diff 29935

tests/run-tests.py

scripts_dir = os.path.join(*scripts_path) scripts_dir = os.path.join(*scripts_path)
extra_paths.append(scripts_dir) extra_paths.append(scripts_dir)
os.environ['PATH'] = os.pathsep.join(extra_paths + path) os.environ['PATH'] = os.pathsep.join(extra_paths + path)
def _use_correct_mercurial(self): def _use_correct_mercurial(self):
target_exec = os.path.join(self._custom_bin_dir, b'hg') target_exec = os.path.join(self._custom_bin_dir, b'hg')
if self._hgcommand != b'hg': if self._hgcommand != b'hg':
# _usecorrectpython() may have augmented PATH, so don't used `which`
AlphareUnsubmitted
Not Done

/s/used/use/

Alphare: /s/used/use/
# on the basename of the explicitly provided executable, since that
# may end up using another one.
#
# TODO: figure how why test-run-tests.t fails on Windows if we
# unconditionally join with bindir, and run with --local. Not
# using --local will run the *.t fine.
if self.options.with_hg:
real_exec = os.path.join(self._bindir, self._hgcommand)
else:
# shutil.which only accept bytes from 3.8 # shutil.which only accept bytes from 3.8
real_exec = which(self._hgcommand) real_exec = which(self._hgcommand)
if real_exec is None: if real_exec is None:
raise ValueError('could not find exec path for "%s"', real_exec) raise ValueError('could not find exec path for "%s"', real_exec)
if real_exec == target_exec: if real_exec == target_exec:
# do not overwrite something with itself # do not overwrite something with itself
return return
if WINDOWS: if WINDOWS:
with open(target_exec, 'wb') as f: with open(target_exec, 'wb') as f:
f.write(b'#!/bin/sh\n') f.write(b'#!/bin/sh\n')