EFForg / https-everywhere

A browser extension that encrypts your communications with many websites that offer HTTPS but still allow unencrypted connections.
https://eff.org/https-everywhere
Other
3.37k stars 1.09k forks source link

Test suite fails on Ubuntu #20090

Closed clasick closed 3 years ago

clasick commented 3 years ago

Type: Testing

Testing on a fresh Ubuntu 20.04 machine. The install dependencies script passes with no errors or warnings. The Firefox test passes as well, but the Chrome test alone fails with the following error:

+ rm -r /tmp/tmp.quFjPAMjlj
+ ./test/chromium.sh
+ '[' -n '' ']'
+ git rev-parse
++ git rev-parse --show-toplevel
+ cd /home/vkk/Projects/https-everywhere
++ which xvfb-run
+ '[' -z '' -a -n /usr/bin/xvfb-run ']'
+ XVFB_RUN=xvfb-run
+ '[' '' == --justrun ']'
+ ./make.sh
Building version 2021.4.15
 * Parsing XML ruleset and constructing JSON library...
 * Writing JSON library to src/chrome/content/rules/default.rulesets and src/chrome/content/rules/default.rulesets.json
 * Everything is okay.
/usr/bin/chromium-browser
CWS crx package has sha256sum: 7b8fc8dc9295d75203b023999734c3c745841463e39ba88723b3c893498a
fd56
EFF crx package has sha256sum: b65187eb3044b07c798dee9f107a816b41873158e41598ca70eb065da0b5
4556
AMO xpi package has sha256sum: 864636b2179356ad99eacfcc34ae26fc3b7de35b6f475c99949ce32618ad
d669
EFF xpi package has sha256sum: 27e1882f791982bb5d7d65b8d783e21c98d54eed6ef2c49156eac6ec08f2
3f77
Total included rules: 23766
Rules disabled by default: 5196
Created pkg/https-everywhere-2021.4.15~pre-amo.xpi
Created pkg/https-everywhere-2021.4.15~pre-eff.xpi
Created pkg/https-everywhere-2021.4.15~pre-cws.crx
Created pkg/https-everywhere-2021.4.15~pre-eff.crx
+ echo 'running tests'
running tests
++ ls -tr pkg/https-everywhere-2021.4.15~pre-cws.crx pkg/https-everywhere-2021.4.15~pre-eff.crx
++ tail -1
+ CRX_NAME=pkg/https-everywhere-2021.4.15~pre-eff.crx
+ xvfb-run python3 test/script.py Chrome pkg/https-everywhere-2021.4.15~pre-eff.crx

Chrome: HTTP to HTTPS redirection failed

Are there any additional steps to be done before running the test suite other than installing dependencies or is the suite just broken in general?

zoracon commented 3 years ago

This is something I also experience locally. I will investigate.

clasick commented 3 years ago

This is something I also experience locally. I will investigate.

Let me know if you need any help debugging the issue or validating a potential fix. 🤠

zoracon commented 3 years ago

This is something I also experience locally. I will investigate.

Let me know if you need any help debugging the issue or validating a potential fix. cowboy_hat_face

Upon looking into test/script.py, Firefox allows the needed load time for the extension and it seems Chrome now needs that same caveat. If you place: time.sleep(1) in the same place as where the Firefox section has, It loads and then redirects properly. Which calls https://github.com/EFForg/https-everywhere/issues/11793, where it seems we were never sure why this was needed. I will see if I can investigate if this is a race condition of Selenium webdriver or considered environmental on browser load times and conditions.

So there's a temporary solution I can place if I do not find out the real problem in a timely fashion.

clasick commented 3 years ago

This is something I also experience locally. I will investigate.

Let me know if you need any help debugging the issue or validating a potential fix. cowboy_hat_face

Upon looking into test/script.py, Firefox allows the needed load time for the extension and it seems Chrome now needs that same caveat. If you place: time.sleep(1) in the same place as where the Firefox section has, It loads and then redirects properly. Which calls #11793, where it seems we were never sure why this was needed. I will see if I can investigate if this is a race condition of Selenium webdriver or considered environmental on browser load times and conditions.

So there's a temporary solution I can place if I do not find out the real problem in a timely fashion.

Thanks for the update! Adding time.sleep(1) makes the test pass. It must definetely be a driver initialization race condition as you've mentioned.