Add helpers to TestPipResult

[pradyunsg]

Supercedes #7869, unblocking the rest of the work, toward updating all the call sites in our tests.

[deveshks]

There is already #8295 posted for this.

[ssurbhi560]

@deveshks I will go on and close those PRs and as @pradyunsg mentioned here we will go on and work on using the helpers on pip's tests. :)

[pradyunsg]

Whhops! I definitely missed that PR (sorry!).

None the less, I think @ssurbhi560 resolved the overlapping PRs conflict (thanks!) so, there's not really much for me to say or do here now.

[pradyunsg]

@ssurbhi560 @gutsytechster Your turn. ;)

[gutsytechster]

Thanks, @pradyunsg for the merge. :)
@ssurbhi560 let me know which files are you updating?

[ssurbhi560]

@gutsytechster I haven't yet started updating any files, so it'd be better if you let me know which ones you have already updated (and going to do) and then I will start working on remaining ones!

[gutsytechster]

@ssurbhi560 as I can find, we have these 11 files to be updated:

• tests/functional/test_download.py
• tests/functional/test_install.py
• tests/functional/test_install_compat.py
• tests/functional/test_install_extras.py
• tests/functional/test_install_index.py
• tests/functional/test_install_reqs.py
• tests/functional/test_install_upgrade.py
• tests/functional/test_install_vcs_git.py
• tests/functional/test_install_wheel.py
• tests/functional/test_uninstall.py
• tests/functional/test_wheel.py

I've taken reference from your previous PR https://github.com/pypa/pip/pull/7869/files. We can distribute the files, let's say you may go with the first 5 and I can go with the rest of them. Does it make sense?

PS: Please mention any other file in case I might've not noticed or mentioned.

[ssurbhi560]

@gutsytechster sounds perfect to me!
Also I went on to check and found there a few more files we would need to update :

tests/functional/test_new_resolver_user.py
tests/functional/test_install_user.py
tests/functional/test_uninstall_user.py
tests/functional/tests_install_force_reinstall.py

So, I would go on and work with the first five from the list you mentioned above and the first two from the list of remaining files. Does this sounds good?

[gutsytechster]

Sounds like a plan to me :) 👍

[xavfernandez]

I'm a little late but I find that those helpers' names lack an assert part.
Was assert_did_create considered ?

[pradyunsg]

Was assert_did_create considered ?

It was... I figured that it wasn't necessary. If you reckon it'd be helpful, I'm happy to file a PR myself after all the transitory PRs are merged. :)

[xavfernandez]

I reckon it'd be helpful but if I'm the only one, I'll adapt.

[pfmoore]

I haven't been following the discussion, but I'd also be mildly in favour of having assert in the name. As it stands, my instincts are to write assert result.did_create(...). Again, it's not a big deal to me, but I wanted to note that @xavfernandez isn't the only one...

[sbidoul]

Same feeling as @xavfernandez here. To me did_create reads like a function that returns a boolean.

[sbidoul]

(sorry, I did not mean to insist, the last comment was not refreshed in my browser tab)

[pradyunsg]

Alrighty. Let's do the {did_* -> assert_did_*} renaming once we merge the corresponding PRs for adopting these helpers (see PRs crosslinked by GitHub, near the end of #6050 currently). :)