Skip to content

Fix some problems found if no MS compiler at all#3231

Merged
bdbaddog merged 2 commits into
SCons:masterfrom
mwichmann:mdw-no-cl-syntax
Nov 7, 2018
Merged

Fix some problems found if no MS compiler at all#3231
bdbaddog merged 2 commits into
SCons:masterfrom
mwichmann:mdw-no-cl-syntax

Conversation

@mwichmann

Copy link
Copy Markdown
Collaborator

A few tests blew up with exceptions (AttributeError, IndexError) if no compiler is installed on Windows - from where they are it could possibly happen on other platforms as well. Changes are to the test harness.

Signed-off-by: Mats Wichmann mats@linux.com

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated master/src/CHANGES.txt directory (and read the README.txt in that directory)
  • I have updated the appropriate documentation

A few tests blew up with exceptions (AttributeError, IndexError) if no
compiler is installed on Windows - from where they are it could possibly
happen on other platforms as well.

Signed-off-by: Mats Wichmann <mats@linux.com>
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.8%) to 79.073% when pulling d5e8a04 on mwichmann:mdw-no-cl-syntax into b70ca92 on SCons:master.

@coveralls

coveralls commented Oct 30, 2018

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 79.917% when pulling d5e8a04 on mwichmann:mdw-no-cl-syntax into b70ca92 on SCons:master.

return None
result = env.WhereIs(prog)
if norm and os.sep != '/':
if result and norm and os.sep != '/':

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add to the docstring for detect what the arguments do?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like Norm should default to False and is a boolean flag to say whether to normalize the path?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't look at norm... will do so. just added the check that if env.WhereIs didn't have a good result, then doing replace on it throws error

AttributeError: 'NoneType' object has no attribute 'replace'

I'll update docstring.

return self._stdout[run]
try:
return self._stdout[run]
except IndexError:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would this happen? Can you add a comment?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had an exception... I guess there's no contents to self._stdout at all, so can't index into it.

493/1179 (41.82%) C:\\Python37\\python.exe -tt test\Interactive\implicit-VariantDir.py
timed out waiting for C:\Users\mats\AppData\Local\Temp\testcmd.6464.8hocmiyx\1 to exist

Traceback (most recent call last):
  File "test\Interactive\implicit-VariantDir.py", line 106, in <module>
    test.wait_for(test.workpath('1'))
  File "C:\users\mats\Documents\github\scons\testing\framework\TestSCons.py", line 1375, in wait_for
    stdout = self.stdout()
  File "C:\users\mats\Documents\github\scons\testing\framework\TestCmd.py", line 1593, in stdout
    return self._stdout[run]
IndexError: list index out of range

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does your change skip the exception but fail the test if it should fail?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm.... doesn't look like there's a case for what happens. not sure what should happen.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that it's merged... implicit-VariantDir passes, which maybe it shouldn't? The two from the other test, sconsign/script/Configure.py and dblite.py (same dir) both pass now.

Signed-off-by: Mats Wichmann <mats@linux.com>
@bdbaddog bdbaddog merged commit fd0ee66 into SCons:master Nov 7, 2018
@mwichmann mwichmann deleted the mdw-no-cl-syntax branch November 8, 2018 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants