Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make tox pass selenium environment variables #1892

Merged
merged 1 commit into from Mar 11, 2024

Conversation

living180
Copy link
Contributor

Description

When running tox, pass through the user's DISPLAY and DJANGO_SELENIUM_TESTS environment variables, so that

DJANGO_SELENIUM_TESTS=true tox

will actually run the Selenuim integration tests. Without this change, the test suite never sees the DJANGO_SELENIUM_TESTS variable and thus skips the integration tests. Without DISPLAY, the integration tests will error out (unless CI is present in the environment to instruct the test suite to run the Selenium webdriver in headless mode).

Checklist:

  • I have added the relevant tests for this change.
  • I have added an item to the Pending section of docs/changes.rst.

When running tox, pass through the user's DISPLAY and
DJANGO_SELENIUM_TESTS environment variables, so that

    DJANGO_SELENIUM_TESTS=true tox

will actually run the Selenuim integration tests.  Without this change,
the test suite never sees the DJANGO_SELENIUM_TESTS variable and thus
skips the integration tests.  Without DISPLAY, the integration tests
will error out (unless CI is present in the environment to instruct the
test suite to run the Selenium webdriver in headless mode).
@tim-schilling
Copy link
Contributor

I think we previously left the selenium tests off because they're a bit burdensome to run locally.

Copy link
Member

@matthiask matthiask left a comment

Choose a reason for hiding this comment

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

Makes sense. This should make it easier to run the selenium tests locally.

@tim-schilling tim-schilling merged commit cfd4801 into jazzband:main Mar 11, 2024
26 checks passed
@living180
Copy link
Contributor Author

Yes, this PR isn't changing that default, it is just making it possible to enable them through tox if desired. Right now the Contributing docs includes the following snippet:

image

The first two lines using make work, but the third using tox doesn't without this change, since tox won't pass through the DJANGO_SELENIUM_TESTS variable without it.

@living180 living180 deleted the tox_selenium branch March 11, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants