search

LibreCat / Catmandu-SRU

Catmandu module for working with SRU data.
https://metacpan.org/release/Catmandu-SRU
5 stars 5 forks source link

Disable HTTP proxy in t/Catmandu-Fix-search_sru.t #45

Closed rbalint closed 3 years ago

rbalint commented 4 years ago

Ubuntu's autopkgtest infrastructure sets it, but not for accessing localhost.

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.3%) to 89.418% when pulling b983f03262383803d0fcebb12ecc83e38270b266 on rbalint:dev into 1ed2ac344656c1c8f429fc240c41de6ed2c562ba on LibreCat:dev.

jorol commented 4 years ago

Thanks for your pull request. I am a bit hesitant to merge it, because I don't like the idea to override system environment variables in a test script. This test was successfully executed on many different machines and OS without any problems, see http://matrix.cpantesters.org/?dist=Catmandu-SRU+0.428. @nics, @phochste, @Corion: what do you think?

Corion commented 4 years ago

If this is for a test with a local HTTP server, one should remove proxies. Test::HTTP::LocalServer (claims to) clear the relevant/interfering environment variables, so this change should not be necessary.

If this is necessary, there are at least two more environment variables to clean out:

$ENV{ HTTP_PROXY }
$ENV{ HTTPS_PROXY }
phochste commented 4 years ago

In general don't see an immediate harm overwriting environment variables in a test script. It has no effect outside the test environment, only within the execution context of that test script. I don't know about this specific case if it is needed and follow @Corion.