Skip to content

ci: Switch to using geckodriver#6541

Merged
rjsparks merged 27 commits into
ietf-tools:mainfrom
larseggert:ci-selenium-geckodriver
Nov 21, 2023
Merged

ci: Switch to using geckodriver#6541
rjsparks merged 27 commits into
ietf-tools:mainfrom
larseggert:ci-selenium-geckodriver

Conversation

@larseggert

Copy link
Copy Markdown
Collaborator

Because that works also on amd64.

@larseggert larseggert changed the title ci: Switch to using geckdriver ci: Switch to using geckodriver Oct 25, 2023
@larseggert
larseggert force-pushed the ci-selenium-geckodriver branch from 6de85b5 to 75c9868 Compare October 25, 2023 18:25
@codecov

codecov Bot commented Oct 25, 2023

Copy link
Copy Markdown

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (4500642) 88.82% compared to head (90646ff) 88.83%.
Report is 10 commits behind head on main.

Files Patch % Lines
ietf/utils/jstest.py 50.00% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6541   +/-   ##
=======================================
  Coverage   88.82%   88.83%           
=======================================
  Files         285      285           
  Lines       40230    40224    -6     
=======================================
- Hits        35734    35732    -2     
+ Misses       4496     4492    -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@larseggert
larseggert marked this pull request as ready for review October 25, 2023 22:24

@jennifer-richards jennifer-richards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to me (and tests run, though with a couple failures, on my m2 mac)

@jennifer-richards
jennifer-richards self-requested a review October 26, 2023 20:18

@jennifer-richards jennifer-richards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Withdrew my approval - noticed that the geckodriver tests didn't actually run here. I think it pulls the standard base image, so doesn't have geckodriver installed. Testing locally, there are some changes needed.

The inputs are hidden and managed by JS, so click
the visible elements instead.
Waiting for the button to be clickable does not
work because the modal is still fading in, so does
not actually close. Would be better to check for it
responding, but I haven't found the right way to do
that yet.
@jennifer-richards

Copy link
Copy Markdown
Member

Those commits fix the selenium tests under ietf.meeting.tests_js for me (apologies for the sleep() in the second one, but I haven't found a better way)

@jennifer-richards

jennifer-richards commented Oct 26, 2023

Copy link
Copy Markdown
Member

Still a failure in ietf.doc.tests_js.EditAuthorsTests.test_add_author_forms

With the last commit, all tests pass for me.

Sending '\n' does not seem to work as it did before,
so click the option instead.

Also reverted some fixme hacks that seem to be obe.
@jennifer-richards
jennifer-richards dismissed their stale review October 27, 2023 03:31

Selenium tests didn't run

@rjsparks rjsparks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this to draft until it passes in both CI and on both mac and windows developer's platforms

@larseggert

Copy link
Copy Markdown
Collaborator Author

You need to rebuild the base docker image locally first to have geckodriver available in the dev image.

@jennifer-richards

Copy link
Copy Markdown
Member

You need to rebuild the base docker image locally first to have geckodriver available in the dev image.

That's what I've done locally (mac). Can you check whether the tests I touched work on your system? (I'm a little worried that I may have undone workarounds that are platform dependent in my purported fixes)

@larseggert
larseggert marked this pull request as draft October 28, 2023 00:29
@jennifer-richards

Copy link
Copy Markdown
Member

For me (m2 mac, geckodriver 0.33.0 (a80e5fd61076 2023-04-02 18:31 +0000)):

dev ➜ /workspace $ ietf/manage.py test --settings=settings_test --skip-coverage ietf.meeting.tests_js.InterimTests
     Datatracker 11.0.0-dev test suite, 30 October 2023 12:05:35 PDT:
     Python 3.9.18 (main, Oct 12 2023, 02:21:43)  [GCC 10.2.1 20210110].
     Django 4.2.6, settings 'settings_test'
     Changing TEMPLATES[0]['OPTIONS']['string_if_invalid'] to '' during testing
     Changing INTERNAL_IPS to '[]' during testing.
     Running an SMTP test server on 127.0.0.1:2026 to catch outgoing email.
     Saving factory-boy random state to .factoryboy_random_state
     Validating all HTML generated during the tests
Found 12 test(s).
     Creating test database...
     Loading global test fixtures: names, ietf.utils.test_data.make_immutable_base_data, nomcom_templates, proceedings_templates
System check identified no issues (3 silenced).
............
----------------------------------------------------------------------
Ran 12 tests in 90.344s

OK

I previously ran the whole suite (without the --skip-coverage) without failure as well. Maybe it's a time-of-day or date-related thing? Or something to do with orbstack vs docker? (seems unlikely)

@larseggert
larseggert marked this pull request as ready for review November 4, 2023 13:35
@larseggert

Copy link
Copy Markdown
Collaborator Author

Tests passed now...

@jennifer-richards

Copy link
Copy Markdown
Member

Tests are still passing for me on my M2. Still have the bootstrapping problem for running the selenium tests on github CI, though.

@NGPixel

NGPixel commented Nov 8, 2023

Copy link
Copy Markdown
Member

geckodriver added to the base image.

@rjsparks

Copy link
Copy Markdown
Member

What can we do to get insight into why geckodriver is crashing in the test container?

@NGPixel

NGPixel commented Nov 13, 2023

Copy link
Copy Markdown
Member

@rjsparks Use the tests debug script:

  1. Clone the PR locally (gh pr checkout 6541)
  2. cd to dev/tests
  3. run sh debug.sh
  4. after init, run:
./ietf/manage.py check
./ietf/manage.py migrate --fake-initial
./ietf/manage.py test -v2 --validate-html-harder --settings=settings_test

While this will run in the same environment setup as GHA, including the container being limited to 2 cores and 7GB of RAM, you'll be running in arm64, while GHA is using x64. So if you don't get the errors, a x64 environment would be needed to properly reproduce.


I was not able to reproduce the crash on my side...

@larseggert

larseggert commented Nov 13, 2023

Copy link
Copy Markdown
Collaborator Author

What's in geckodriver.log when it crashes?

@NGPixel

NGPixel commented Nov 13, 2023

Copy link
Copy Markdown
Member

@larseggert https://github.com/ietf-tools/datatracker/suites/18134236487/artifacts/1045634129

The recurring error being:

1699868894816	mozrunner::runner	INFO	Running command: MOZ_CRASHREPORTER="1" MOZ_CRASHREPORTER_NO_REPORT="1" MOZ_CRASHREPORTER_SHUTDOWN="1" MOZ_NO_REMOTE="1" "/github/home/.cache/selenium/firefox/linux64/119.0.1/firefox" "--marionette" "--headless" "--remote-debugging-port" "51209" "--remote-allow-hosts" "localhost" "-no-remote" "-profile" "/tmp/rust_mozprofileOPgBo4"
Running Firefox as root in a regular user's session is not supported.  ($HOME is /github/home which is owned by uid 1001.)

@NGPixel

NGPixel commented Nov 13, 2023

Copy link
Copy Markdown
Member

Adding HOME=/root before running the tests appears to be working.

This is a long standing firefox specific bug that they can't be bothered to fix.

@rjsparks
rjsparks merged commit 3d44825 into ietf-tools:main Nov 21, 2023
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Nov 25, 2023
@larseggert
larseggert deleted the ci-selenium-geckodriver branch December 18, 2023 08:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants