search

openzim / python-scraperlib

Collection of Python code to re-use across Python-based scrapers
GNU General Public License v3.0
19 stars 16 forks source link

Reactivate the `test_user_agent` test #129

Closed benoit74 closed 7 months ago

benoit74 commented 7 months ago

We had an issue with the test_user_agent test: https://github.com/openzim/python-scraperlib/actions/runs/7869193867/job/21467889432?pr=128

Looks like a transient name resolution error in Github Actions, but we had to disable the test until then.

It should be reactivated asap.

rgaudin commented 7 months ago

That doesn't sound like le proper way to handle such an issue. If it's a network issue then your tests are red and you mention why in your PR. If it's not resolved after a reasonable time, the test must be rewritten.

I don't see how removing a test will help. Just to get green checks? But if you remove a test, chances are that coverage will decrease…

benoit74 commented 7 months ago

I feel like red CI is worse than a fully red CI which is hard to diagnose and not reporting coverage at all. And blocking PRs.

I agree that this has to be fixed in the medium term, and this is why I opened this issue (and intended to open a PR to be able to regularly check if situation is back to normal).

rgaudin commented 7 months ago

We are talking about DNS error. It's already working again. I don't understand your reasoning here.

kelson42 commented 7 months ago

General comment: CI should be green. If not, code should be fixed, either in software or test suite. Obviously, if this work is blocking a very urgent thing (strong biz constraint, incident ongoing, ...), then we handle as exception... but only in that case.