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 integration tests against deployment #2962

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

soapy1
Copy link
Contributor

@soapy1 soapy1 commented Feb 14, 2025

Reference Issues or PRs

#2855, #2950

What does this implement/fix?

This PR updates the integration tests to support running against an existing cluster. You can do this by providing the EXISTING_DEPLOYMENT_DIR env var (opposed to the --cloud flag) when running the integration tests.
EXISTING_DEPLOYMENT_DIR gets a path to an existing nebari config dir (must contain the nebari-config.yaml and the output dir /stages). For example see How to test this PR?

Also adds these tests to the Local Integration Tests GHA workflow.

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

How to test this PR?

Create a local deploy of nebari and then run the integration tests.

$ mkdir nebari-local
$ nebari init ....
$ nebari deploy ....
$ cd ./tests/tests_integration
$ EXISTING_DEPLOYMENT_DIR=../../nebari-local pytest -vvv -s .

@soapy1 soapy1 force-pushed the run-integration-tests-against-deployment branch 3 times, most recently from f1e7883 to f857acc Compare February 14, 2025 23:31
@marcelovilla
Copy link
Member

@soapy1 I know this is still a draft PR, but I wanted to check if you’re planning to address #2950. If so, it might be a good opportunity to offload as much cloud deployment logic as possible from pytest.

Let me know what you think!

@soapy1
Copy link
Contributor Author

soapy1 commented Feb 20, 2025

@marcelovilla yep, definitely can do. Just to clarify, you mean like update the ci workflows (eg. test_azure_integration.yaml) to use the change proposed by this PR?

@marcelovilla
Copy link
Member

@soapy1 I was actually referring to the --cloud flag and the deployment fixtures, which will no longer be needed once we address #2950 (which seems to overlap with—or potentially duplicate—#2855). That said, we don’t need to remove them in this PR. I just wanted to bring it up in case it impacts your approach, especially if working around them requires extra effort when they might not exist in the future.

Regarding the use of the changes proposed in this PR for CI workflows in cloud integration tests, ideally, it would be great to have both local and cloud integration tests behave consistently, with the only difference being the deployment/destroy process. In other words, it would be beneficial if our tests could run against any Nebari cluster, regardless of how it was deployed. I’m not sure whether incorporating the changes from this PR into those CI workflows is the best approach or if we’d be better off having a separate workflow that allows running tests against any deployment. That is beyond the scope of this PR, but maybe it's worthwhile to keep it mind for the future.

Let me know if this makes sense!

@soapy1 soapy1 force-pushed the run-integration-tests-against-deployment branch 5 times, most recently from 1dc3f8b to 1dfc35e Compare February 26, 2025 22:50
@soapy1 soapy1 force-pushed the run-integration-tests-against-deployment branch from a3a0baf to f2e7ed0 Compare February 27, 2025 00:44
@soapy1
Copy link
Contributor Author

soapy1 commented Feb 27, 2025

@marcelovilla looks like tests are passing and this is setup correctly. This PR will make the test_<cloud>_integration.yaml tests fail.
It looks like it might be ok to run these integration tests as part of the test-provider.yaml workflow. Do you have any preferences on how to use these changes to test in ci?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New 🚦
Development

Successfully merging this pull request may close these issues.

2 participants