-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
changes to unit test to avoid hitting the filesystem #1
Conversation
@@ -322,12 +361,12 @@ def parseToken(self, token): | |||
# 'Foo$1Inner' | |||
# https://github.com/SCons/scons/issues/2087 | |||
if self.outer_state.localClasses and \ | |||
self.outer_state.stackBrackets[-1] > \ | |||
self.outer_state.stackBrackets[-2]+1: | |||
self.outer_state.stackBrackets[-1] > \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe parens here to group the "a > b" part would make it more readable? and eliminate the need for that backslash-line-continuation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does pylint think of such? ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No particular reaction one way or the other. . Has plenty of other things to say, though... included just for grins:
This change adds support for scanning multiple entries in an action string in order to better support the following use cases: 1. A file is provided in an action string and should be taken as a dependency. For example, an action string "$PERL somefile.pl". 2. An action string actually has two actions separated by &&. For example, "cd <some_dir> && $ZIP <args>". Adding support for #1 actually allows us to fix the test IMPLICIT_COMMAND_DEPENDENCIES.py on Windows, which was previously treating a Python file as executable even on Windows. This was causing tests to repeatedly open the default handler of Python files, which if set to Visual Studio causes DDE hangs. This test is fixed because now we can have the action string specify python as the first command and still take an implicit dependency on the script, which is now the second command.
The way runtest.py passes the list of fixture directories is racy because it sets it in os.environ['FIXTURE_DIRS'] and then spawns the subprocess, counting on Python to start the subprocess before that list is overwritten when spawning the next directory. At least on Windows, the environment is not copied in subprocess.run so runtest.py may overwrite the list of fixture directories with the list for test #2 while the subprocess module is still kicking off test #1. I was able to easily reproduce this by running the command: `python runtest.py -j 2 test\MSVC\VSWHERE.py test\AS\ASPPFLAGS.py` a few times in a row. However, with this fix, that command repeatedly succeeds. To validate ths fix, I also ran that command with "--xml a.xml" and "--xml a.xml --nopipefiles" to validate that those other executors worked correctly.
Slight reorg of setting default
No description provided.