Skip to content

Cleanup UI integration tests #2055

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

Merged
merged 5 commits into from
Dec 13, 2022
Merged

Cleanup UI integration tests #2055

merged 5 commits into from
Dec 13, 2022

Conversation

saeedfazal
Copy link
Contributor

@saeedfazal saeedfazal commented Dec 9, 2022

The change looks too big just because of the indentation of the code due to deletion of outer most ginkgo container in acceptance test files.

  • Deleted UI integration test directory
  • Deleted extra ginkgo Describe context for all specs
  • Deleted all testrunner references and updated them with standard function calls
  • Revisited all test labels and removed integration labels as they are no longer required.
  • Changed test-suite public functions to private as they are not referenced outside of package anymore
  • Updated all global/local variables to adhere to the camel case scheme

Deploy: https://github.com/weaveworks/weave-gitops-enterprise/actions/runs/3658784672
Nightly: https://github.com/weaveworks/weave-gitops-enterprise/actions/runs/3659415510

@saeedfazal saeedfazal added exclude from release notes Use this label to exclude a PR from the release notes team/pesto labels Dec 9, 2022
@saeedfazal saeedfazal self-assigned this Dec 9, 2022
@saeedfazal saeedfazal marked this pull request as draft December 9, 2022 16:20
@saeedfazal saeedfazal marked this pull request as ready for review December 9, 2022 16:20
@saeedfazal saeedfazal requested a review from foot December 9, 2022 16:21
@saeedfazal saeedfazal force-pushed the cleanup_ui_integration_tests branch 2 times, most recently from e7f0a9d to b955867 Compare December 12, 2022 09:28
@saeedfazal saeedfazal force-pushed the cleanup_ui_integration_tests branch from b955867 to 30be3f5 Compare December 13, 2022 10:31
@@ -379,6 +380,113 @@ func verifyCapiClusterHealth(kubeconfigPath string, applications []Application)
}
}

// This function generates multiple template files from a single template to be used as test data
func generateTestTemplates(templateCount int, templateFile string) (templateFiles []string, err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did we remove some of the template "go-templates" recently? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, Now we have number of types of gitopsTemplate. Almost all tests use unique templates to test rather than multiple copies of same test.
We still have support of generating multiple templates form the templated template.
There is still one test which generate 50 templates from one template. It is still supported.

Copy link
Collaborator

@foot foot left a comment

Choose a reason for hiding this comment

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

🧹 !

Great to go back and do some cleanup, thanks for making time for this.

@saeedfazal saeedfazal merged commit 6b8f82d into main Dec 13, 2022
@saeedfazal saeedfazal deleted the cleanup_ui_integration_tests branch December 13, 2022 10:45
AsmaaNabilBakr pushed a commit that referenced this pull request Dec 14, 2022
* Deleted UI integration test directory

* Deleted runner Describe from all specs

* Delete all testrunner references and updated with standard function calls

* Changed test-suite puplic functions to private as they are not referenced outside of package

* Updated acceptance test lables because no more UI integration exclusion is needed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from release notes Use this label to exclude a PR from the release notes team/pesto
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants