Skip to content

Conversation

@mwichmann
Copy link
Collaborator

Code inspection reveals this problem (4 instances):
Class 'OSError' does not define 'getitem' so the [] operator cannot be used on its instances.
Use e.errno for e[0] and e.strerror for e[1].

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

Contributor Checklist:

Tests/docs N/A

  • 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

Code inspection reveals this problem (4 instances):
Class 'OSError' does not define '__getitem__' so the [] operator cannot be used on its instances.
Use e.errno for e[0] and e.strerror for e[1].

Signed-off-by: Mats Wichmann <mats@linux.com>
@bdbaddog
Copy link
Contributor

Do any tests exercise this bit of code?
If not, how hard to create a test to do so?

@mwichmann
Copy link
Collaborator Author

You would have to make a test which forces an OSError here. I'll look at that. Meanwhile, it looks like I missed one of the four instances, so there will be another version coming anyway.

@mwichmann
Copy link
Collaborator Author

This looks like it ought to be in the realm of the SPAWN.py test, but I don't quite see a clear path - that test seems to explore changing the SPAWN construction variable, which means it wouldn't go through the possibly broken one.

@bdbaddog
Copy link
Contributor

You could do this in a unit test say "src/engine/SCons/Platform/win32Tests.py" ?

@mwichmann
Copy link
Collaborator Author

Not sure I'm smart enough to write that test. The problem happens in the function which implements PSPAWN, which is not publicly documented, but looks like it's SPAWN with additions to let us capture the streams. It looks like that is only touched by the TryBuild in SConf.py (I guess this is the public Context.TryBuild?), which in turn is called by the other related checks. There is a Test_TryBuild in SConfTests.py but it doesn't look designed to do negative tests - that is, what happens if you try an operation that is expected to fail with OSError. And the tests of SPAWN/PSPAWN all try to make sure the right thing happens if you redefine them. I'm open to guidance...

@bdbaddog bdbaddog merged commit c594a4d into SCons:master Aug 31, 2018
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.

2 participants