Skip to content

bpo-36638: fix WindowsLoadTracker exception on small windows SKUs - #12849

Merged
zooba merged 6 commits into
python:masterfrom
paulmon:load-tracker-exception
Apr 18, 2019
Merged

bpo-36638: fix WindowsLoadTracker exception on small windows SKUs#12849
zooba merged 6 commits into
python:masterfrom
paulmon:load-tracker-exception

Conversation

@paulmon

@paulmon paulmon commented Apr 16, 2019

Copy link
Copy Markdown
Contributor

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

@ammaraskar ammaraskar left a comment

Copy link
Copy Markdown
Member

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

zooba commented Apr 16, 2019

Copy link
Copy Markdown
Member

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

paulmon commented Apr 16, 2019

Copy link
Copy Markdown
Contributor Author

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

zooba commented Apr 17, 2019

Copy link
Copy Markdown
Member

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

paulmon commented Apr 17, 2019

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

Comment thread Lib/test/libregrtest/main.py Outdated
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)))

@ammaraskar ammaraskar Apr 17, 2019

Copy link
Copy Markdown
Member

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
Copy Markdown
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