bpo-36670: fixed encoding issue with libregrtest on win systems#15488
bpo-36670: fixed encoding issue with libregrtest on win systems#15488LorenzMende wants to merge 1 commit intopython:masterfrom
Conversation
detailed: fixed decoding of typeperf in regrtest for win environments, added lookup for locations dependent typeperf key
| if line.strip() == '' or '\\\\' in line or len(toks) != 2: | ||
| continue | ||
|
|
||
| load = float(toks[1].replace('"', '')) |
There was a problem hiding this comment.
While you're here, could you handle the case where float raises a ValueError (or maybe a TypeError? But I think we're guaranteed to only get ValueErrors at this point)? We can continue to the next line in that case.
There was a problem hiding this comment.
Hi zooba,
what about
| load = float(toks[1].replace('"', '')) | |
| try: | |
| toks = line.strip().split(',') | |
| load = float(toks[-1].replace('"', '')) | |
| except: | |
| continue |
this would handle the ValueError/ TypeError as well as take care of the line parsing.
Or is the bare exception handling unfavourable?
There was a problem hiding this comment.
@LorenzMende That looks like a good option to me. You'll have to merge the change yourself though - I don't have permissions to your PR branch.
|
If it's not too much work, could you create a PR with your alternative solution from bpo whereby you raise early and the load average calculation gets disabled. This really feels like a lot of extra complexity for not much gain, the primary reason system load averages are reported in regrtest is to track down flaky parallel/race condition/non deterministic tests. Those are primarily run on CI machines and build-bots which are almost always English machines anyway |
|
I merged PR #16511 which is contains a similar fix, but also multiple other regrtest bugfixes. |
6 participants
detailed: fixed decoding of typeperf in regrtest for win environments, added lookup for locations dependent typeperf key
https://bugs.python.org/issue36670