Conversation
b0e4a75 to
f381a31
Compare
Changelogs: * https://github.com/pex-tool/pex/releases/tag/v2.74.0 * https://github.com/pex-tool/pex/releases/tag/v2.74.1 * https://github.com/pex-tool/pex/releases/tag/v2.74.2 * https://github.com/pex-tool/pex/releases/tag/v2.74.3 * https://github.com/pex-tool/pex/releases/tag/v2.75.0 * https://github.com/pex-tool/pex/releases/tag/v2.75.1 * https://github.com/pex-tool/pex/releases/tag/v2.75.2 * https://github.com/pex-tool/pex/releases/tag/v2.76.0 * https://github.com/pex-tool/pex/releases/tag/v2.76.1 * https://github.com/pex-tool/pex/releases/tag/v2.77.0 * https://github.com/pex-tool/pex/releases/tag/v2.77.1 NOTE: First update since pantsbuild#22979
f381a31 to
b5e738f
Compare
|
Supercedes this: #22967 |
|
Or does it... |
My hope was that it does, but on my initial quick scan I had hoped that the issue was from lockfile churn but alas! it was not that smooth. |
| return cast(list[str], get_all_data(rule_runner, pex).info["requirements"]) | ||
|
|
||
|
|
||
| def _normalize_url_req(s: str) -> str: |
There was a problem hiding this comment.
As I was alluding to in the other PR, maybe the problem here is with the tests themselves? As in, they're brittle and based on internals that really shouldn't matter?
There was a problem hiding this comment.
I don't disagree: Two lines down:
Several tests here are brittle and rely on Pex/Pants being on the same packaging version
But as a general principle I found patching the assertion the smaller scope creep over delete/refactor a 200(!?) line test.
(I don't think these tests directly matter for this, but I was today looking at areas where we do have inconsistent requirements parsing bugs #22239 which don't give me great "delete the test" vibes.)
There was a problem hiding this comment.
I meant deleting the brittle assertions not the whole test itself, then as John wrote in my PR - using the output of the parsed requirements as a comparison point.
I mean, doesn't really matter - but I think there's a pretty small chance this gets deleted eventually 🤷🏽
There was a problem hiding this comment.
Ah gotcha. I started with that, but once I got to 3 assertions I felt like it was "bigger than a breadbox" or however that joke goes.
sureshjoshi
left a comment
There was a problem hiding this comment.
Other than the idea of just deleting brittle assertions, the update itself is fine.
Changelogs:
NOTE: First update since #22979