fix(dev): Patch firefox_profile.py from Selenium package #29887
Conversation
|
We can ignore the Ubuntu dev env check. |
scripts/lib.sh
Outdated
| # Remove this block when upgrading the selenium package | ||
| if grep -q "or setting is" "${fx_profile}"; then | ||
| echo "We are patching ${fx_profile}. You will see this message only once." | ||
| sed -i .bak 's/or setting is/or setting ==/' ${fx_profile} |
There was a problem hiding this comment.
Could you instead generate a patch and put it in scripts/patches, then idempotently patch? Patching shouldn't be done like this.
There was a problem hiding this comment.
Actually, lemme take this over and show you what I mean.
There was a problem hiding this comment.
Thanks for the suggestion. WFM
scripts/lib.sh
Outdated
| # Remove this block when upgrading the selenium package | ||
| if grep -q "or setting is" "${fx_profile}"; then | ||
| echo "We are patching ${fx_profile}. You will see this message only once." | ||
| sed -i .bak 's/or setting is/or setting ==/' ${fx_profile} |
There was a problem hiding this comment.
Actually, lemme take this over and show you what I mean.
| # Remove this block when upgrading the selenium package | ||
| if grep -q "or setting is" "${fx_profile}"; then | ||
| echo "We are patching ${fx_profile}. You will see this message only once." | ||
| patch -p0 <scripts/patches/firefox_profile.diff |
There was a problem hiding this comment.
Instead of guarding with the grep, you can idempotently (--forward) apply the patch like this: patch -p0 --forward -r=/dev/null < scripts/patches/firefox_profile.diff. The -r=/dev/null is just to prevent it writing a reject file in the case of failure, but it still exits nonzero.
There was a problem hiding this comment.
Oh hmm, it still exits 1. Never mind.
There was a problem hiding this comment.
requirements-base.txt
Outdated
| @@ -89,5 +89,5 @@ phabricator==0.7.0 | |||
|
|
|||
| # test dependencies, but unable to move to requirements-test until | |||
| # sentry.utils.pytest and sentry.testutils are moved to tests/ | |||
| selenium==3.141.0 | |||
| selenium==3.141.0 # XXX: Remove hack from lib.sh when upgrading | |||
There was a problem hiding this comment.
@armenzg can you comment the PR link/number here for future reference?
|
@armenzg So, this fix doesn't seem to fix the issue when I run in getsentry: |
Running
pytestingetsentrytriggers a bug infirefox_profile.pyfrom the Selenium driver. Upgrading to version 4 is quite involved. This is a stop-gap to prevent distracting engineers when it happens.This is the output when we patch it: