[MAINTENANCE] Run cloud e2e tests. #8443
Conversation
✅ Deploy Preview for niobium-lead-7998 canceled.
|
| @@ -107,8 +107,7 @@ jobs: | |||
| markers: | |||
| - big | |||
| - cli | |||
| # `not e2e` because 1 test & 1 fixture need access token | |||
| - cloud and not e2e | |||
There was a problem hiding this comment.
I will need to update the branch protection rules before this will merge.
Also, after this merges I will need to add the cloud step as a required test.
.github/workflows/ci.yml
Outdated
| @@ -137,6 +136,10 @@ jobs: | |||
| invoke deps --gx-install -m '${{ matrix.markers }}' -r test | |||
|
|
|||
| - name: Run the tests | |||
| env: | |||
There was a problem hiding this comment.
Maybe I should only set these env variables for the cloud mark stage?
There was a problem hiding this comment.
There may be a way to conditionally inject env vars with gh actions but another simpler solution could be to just break the steps that need different env vars to their own job.
There was a problem hiding this comment.
Yeah, that sounds like a good idea.
There was a problem hiding this comment.
I'm going to leave it like this for this PR and move it out later if we want.
There was a problem hiding this comment.
Actually, I will move this out now. Setting the environmental variables cause a lot of issues. I will file a followup ticket to review this further. It only impacts people who are both gx cloud and OSS developers.
| @@ -819,7 +819,8 @@ def test_config_substitution_retains_original_value_on_save( | |||
| from great_expectations import get_context | |||
|
|
|||
| context = get_context( | |||
| context_root_dir=file_dc_config_file_with_substitutions.parent | |||
| context_root_dir=file_dc_config_file_with_substitutions.parent, | |||
| ge_cloud_mode=False, | |||
There was a problem hiding this comment.
I've set the cloud env variables for all marker stages so had to make sure these didn't get a cloud context.
Maybe I should just set the env variables on the cloud marker test? Or maybe I should do both. These tests should be robust to the env variables.
There was a problem hiding this comment.
Doing things like this let's me get rid of that darwin check in __init__.py. Ultimately, making the tests pass independent of a developer's environment is better. Currently linux users would fail these tests if they have gx cloud env variables set. I still see tests dying though because I've set the env variables in all marker tests. Hopefully isn't a pain to clean up as we go.
There was a problem hiding this comment.
I will move and use the fixture I introduced and use that instead of setting the ge_cloud_mode parameter.
tests/validator/test_validator.py
Outdated
| @pytest.fixture() | ||
| def _unset_gx_env_variables(monkeypatch: pytest.MonkeyPatch) -> None: | ||
| for var in GXCloudEnvironmentVariable: | ||
| monkeypatch.delenv(var, raising=False) | ||
|
|
||
|
|
||
| # TODO: There is something wrong with this test. It is trying to mock out cloud but if I don't | ||
| # unset the gx_env_variables (eg if i remove this fixture) this test will fail. |
There was a problem hiding this comment.
I had a similar approach in this abandoned effort from about a month ago.
Except in that case I was injecting dummy Cloud variables for the non e2e tests.
Unsetting the gx_env variables might be something we want to apply to the test session setup???
There was a problem hiding this comment.
We do need to normalize our environment somehow. If I unset these across the board the tests that require these environment variables set will fail. We need a way that is not burdensome to a test writer that gets this correct.
Kilo59
force-pushed
the
m/pp-358/run_cloud_tests
branch
from
09c427e
to
d726393
Compare
invoke lint(usesblack+ruff)For more details, see our Contribution Checklist, Coding style guide, and Documentation style guide.
After you submit your PR, keep the page open and monitor the statuses of the various checks made by our continuous integration process at the bottom of the page. Please fix any issues that come up and reach out on Slack if you need help. Thanks for contributing!