tests: fix test_finder_detects_latest_already_satisfied_pypi_links
#5901
Conversation
benoit-pierre
commented
Oct 19, 2018
benoit-pierre
commented
- use the correct PyPI URL (need HTTPS or it's ignored)
- ensure the PyPI index is actually hit (fail if the network is down)
- use the correct PyPI URL (need HTTPS or it's ignored) - ensure the PyPI index is actually hit (fail if the network is down)
cjerdonek
added
C: tests
|
cc: @uranusjr |
|
Sorry I missed this. What is the failure this intends to fix? |
|
@uranusjr: just fixing the test. |
|
But why does the test need fixing, isn’t it passing now on master? (Sorry if the question sound dumb, I don’t understand the context of this.) |
|
FWIW, I have the same question. |
|
I don't understand why it's not clear from the PR description: the test is not doing it's job, it's passing yes, but more importantly it's not failing when it should. |
|
Okay, thanks. @uranusjr Can you review this since this relates to the code that you've been putting a lot of thought into? |
|
I see, sorry it didn’t seem obvious to me since I am not familiar with what |
| for page in finder_get_pages(locations, project_name): | ||
| get_pages.page_found = True | ||
| yield page | ||
| get_pages.page_found = False |
There was a problem hiding this comment.
Would it be more pragmatic to use the mock’s side_effect for this, instead of mocking the function with a custom one? That way you can also add an assert finder._get_pages.call_count == 1 to make the check more obvious.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
lock
bot
added
the
auto-locked
