Write Customization tests in python #2918
Conversation
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
hubertdeng123
force-pushed
the
hubertdeng123/customization-tests-python
branch
from
11f9bea
to
5c2fd16
Compare
hubertdeng123
force-pushed
the
hubertdeng123/customization-tests-python
branch
from
c93b5e5
to
2ccd1a5
Compare
hubertdeng123
force-pushed
the
hubertdeng123/customization-tests-python
branch
from
bcdccaa
to
82eed1e
Compare
hubertdeng123
force-pushed
the
hubertdeng123/customization-tests-python
branch
from
a4f67e4
to
1799f09
Compare
.github/workflows/test.yml
Outdated
| @@ -16,7 +16,7 @@ defaults: | |||
| jobs: | |||
| e2e-test: | |||
| if: github.repository_owner == 'getsentry' | |||
| runs-on: ubuntu-20.04 | |||
| runs-on: ubuntu-22.04 | |||
There was a problem hiding this comment.
bumped this while at it
There was a problem hiding this comment.
nit: Imo we should split this into a separate PR, since I could imagine cases where we want to only roll this bump back.
| ### pytest-sentry configuration ### | ||
| if [ "$GITHUB_REPOSITORY" = "getsentry/self-hosted" ]; then | ||
| echo "PYTEST_SENTRY_DSN=${{ env.SELF_HOSTED_TESTING_DSN }}" >> $GITHUB_ENV | ||
| echo "PYTEST_SENTRY_DSN=$SELF_HOSTED_TESTING_DSN" >> $GITHUB_ENV |
There was a problem hiding this comment.
This was previously not being picked up correctly
| time.sleep(1) | ||
| else: | ||
| if response.status_code == 200: | ||
| break | ||
| else: | ||
| raise AssertionError("timeout waiting for self-hosted to come up") | ||
|
|
||
| if request.config.getoption("--customizations") == "True": | ||
| os.environ['TEST_CUSTOMIZATIONS'] = "True" | ||
| script_content = '''\ |
There was a problem hiding this comment.
These lines are definitely covered by testing, not sure why Codecov is complaining here.
There was a problem hiding this comment.
That's the reason why Test / integration test (True, v2.0.1) and Test / integration test (True, v2.7.0) are taking 8 minutes longer
There was a problem hiding this comment.
nit: It's pretty hard for a naive observer to figure out what the "True" in these variants means. Maybe we could have values be Customization Enabled vs Customization Disabled rather than True/False?
There was a problem hiding this comment.
Apparently, you can create a custom name for the job. Now it looks like
Test / integration test v2.0.1 - customizations disabled
.github/workflows/test.yml
Outdated
| @@ -16,7 +16,7 @@ defaults: | |||
| jobs: | |||
| e2e-test: | |||
| if: github.repository_owner == 'getsentry' | |||
| runs-on: ubuntu-20.04 | |||
| runs-on: ubuntu-22.04 | |||
There was a problem hiding this comment.
nit: Imo we should split this into a separate PR, since I could imagine cases where we want to only roll this bump back.
| time.sleep(1) | ||
| else: | ||
| if response.status_code == 200: | ||
| break | ||
| else: | ||
| raise AssertionError("timeout waiting for self-hosted to come up") | ||
|
|
||
| if request.config.getoption("--customizations") == "True": | ||
| os.environ['TEST_CUSTOMIZATIONS'] = "True" | ||
| script_content = '''\ |
There was a problem hiding this comment.
nit: It's pretty hard for a naive observer to figure out what the "True" in these variants means. Maybe we could have values be Customization Enabled vs Customization Disabled rather than True/False?
| for command in commands: | ||
| result = subprocess.run(command, check=False) | ||
| if os.getenv("TEST_CUSTOMIZATIONS", "False") == "True": | ||
| assert result.returncode == 0 |
There was a problem hiding this comment.
Does test_customization run in both matrix variants?
There was a problem hiding this comment.
Yes, it does. If customizations enabled, it checks to ensure that they are truly enabled by making sure a dependency can be imported that is included in the custom image (ldap). If not, it checks to ensure that an error is thrown when trying to import a dependency that comes from the custom image.
This PR adds a few things