Skip to content

Conversation

@paulmon
Copy link
Contributor

@paulmon paulmon commented Apr 16, 2019

typeperf.exe is not present on small editions
of windows like Windows IoT Core or nanoserver
This causes WindowsLoadTracker to throw an exception during test initialization.

@ammaraskar @zooba

https://bugs.python.org/issue36638

Copy link
Member

@ammaraskar ammaraskar left a comment

Choose a reason for hiding this comment

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

Whoops, I assumed typeperf would be available on all supported Windows editions. I think this fix might leave some open handles around, the pipe_write_end and self.pipe in win_utils but this might not be a big concern since its just for the duration of the test suite run.

Aside from that, looks good to me.

@paulmon paulmon changed the title bpo-36638: fix WindowsLoadTracker exception on small windows bpo-36638: fix WindowsLoadTracker exception on small windows SKUs Apr 16, 2019
@zooba
Copy link
Member

zooba commented Apr 16, 2019

I'd rather expand the platform check to exclude ARM. That way we'll find out about changes here in the future faster (when it breaks, rather than when we notice a warning).

@paulmon
Copy link
Contributor Author

paulmon commented Apr 16, 2019

This isn't an ARM issue.
typeperf.exe is not available on Windows IoT Core x64, or nanoserver x64 either.

It makes more sense to check for the existence of c:\windows\system32\typeperf.exe, or typeperf.exe on the path, than to limit it to arm. I'm not sure which might be more pythonic. The missing dependency is what is causing the exception, not the processor architecture.

@zooba
Copy link
Member

zooba commented Apr 17, 2019

In that case, handling the exception when it's not found is better. Sorry to send you back to the first commit (but fix up the formatting while you're there - no space between function call (print) and its arguments, and capitalizing product names is totally okay)

@brettcannon brettcannon added type-bug An unexpected behavior, bug, or error OS-windows labels Apr 17, 2019
@paulmon
Copy link
Contributor Author

paulmon commented Apr 17, 2019

No problem. I'm happy to learn.

Alos, are there any VS Code addins or other tools that will help me get the whitespace right before I create the PR?

@ammaraskar
Copy link
Member

Using a linter like pylint will catch trailing whitespace issues like those https://code.visualstudio.com/docs/python/linting (Note that some of the stdlib doesn't conform to pep8, so you might run into some false positives)

If you don't want to use a full blown linter, for that particular issue you could use something like https://marketplace.visualstudio.com/items?itemName=shardulm94.trailing-spaces

except FileNotFoundError as error:
# Windows IoT Core and Windows Nano Server do not provide
# typeperf.exe for x64, x86 or ARM
print('Failed to create WindowsLoadTracker: {}'.format(str(error)))
Copy link
Member

@ammaraskar ammaraskar Apr 17, 2019

Choose a reason for hiding this comment

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

The str() call here is redudant, an empty format specifier is essentially the same as calling str()

A general convention is that an empty format string ("") produces the same result as if you
had called str() on the value. A non-empty format string typically modifies the result.

https://docs.python.org/3/library/string.html#format-specification-mini-language

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@zooba zooba merged commit 264a0b4 into python:master Apr 18, 2019
@paulmon paulmon deleted the load-tracker-exception branch May 23, 2019 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OS-windows skip news type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants