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

Run proxito tests with proxito #6714

Merged
merged 11 commits into from
Mar 10, 2020

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Feb 26, 2020

No description provided.

@@ -213,6 +213,7 @@ def test_project_nginx_serving_unicode_filename(self):
@override_settings(
PYTHON_MEDIA=False,
PUBLIC_DOMAIN='readthedocs.io',
RTD_BUILD_MEDIA_STORAGE='readthedocs.rtd_tests.storage.BuildMediaFileSystemStorageTest',
Copy link
Member Author

Choose a reason for hiding this comment

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

Only this test was expecting to use readthedocs.rtd_tests.storage.BuildMediaFileSystemStorageTest instead of readthedocs.proxito.tests.storage.BuildMediaStorageTest, couldn't figure out how to make it work with the other one

Comment on lines -21 to +24
url = reverse('search')

@pytest.fixture(autouse=True)
def setup(self):
self.url = reverse('search')
Copy link
Member Author

Choose a reason for hiding this comment

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

Had to change this, because pytest evaluates/import the file, in proxito we don't have search (yet)

Comment on lines 21 to +24
commands =
pytest --cov-report= --cov-config {toxinidir}/.coveragerc --cov=. --suppress-no-test-exit-code {posargs:{env:TOX_POSARGS:-m 'not search'}}
/bin/sh -c '\
export DJANGO_SETTINGS_MODULE=readthedocs.settings.test; \
pytest --cov-report= --cov-config {toxinidir}/.coveragerc --cov=. --suppress-no-test-exit-code -m "not proxito" {posargs:{env:TOX_POSARGS:-m "not search and not proxito"}}'
Copy link
Member Author

@stsewd stsewd Feb 26, 2020

Choose a reason for hiding this comment

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

-m "not proxito" {posargs:{env:TOX_POSARGS:-m "not search and not proxito"}}

It's hard to make everyone happy :D. So, -m doesn't combine the multiple -m, but it takes the last -m option, so search tests and proxito tests aren't run by default. And proxito tests won't run for people that wants to run search tests.

Just for clarification, proxito tests won't run with this command, but they will in the command below.

@stsewd stsewd requested a review from a team February 26, 2020 23:12
@ericholscher
Copy link
Member

@stsewd is the approach of using flags and -m options for tests documented anywhere? It seems that should PR should also be updating that documentation. I don't fully understand what the goal here is, so it would be good to make sure we are explaining it in docs, to make sure everyone understands it.

@stsewd
Copy link
Member Author

stsewd commented Mar 10, 2020

@ericholscher docs explaining this at #6764

@stsewd
Copy link
Member Author

stsewd commented Mar 10, 2020

Also, this allows us to not hardcode the urls to readthedocs.proxito.urls and use readthedocsinc.proxito.urls in .com with the same set of tests. With this we can run the tests from the proxied api on .com instead of ignore them.

@stsewd stsewd merged commit 579f241 into readthedocs:master Mar 10, 2020
@stsewd stsewd deleted the run-proxito-tests-with-proxito branch March 10, 2020 01:23
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.

2 participants