crossplane-contrib / provider-argocd

Crossplane provider to provision and manage Argo CD objects
Apache License 2.0
68 stars 35 forks source link

Proposal: add e2e tests based on k8s e2e-framework #89

Closed maximilianbraun closed 2 months ago

maximilianbraun commented 1 year ago

Description of your changes

This PR should act as a proposal for a end-to-end test framework for Crossplane Providers based on kubernetes e2e-framework.

The framework is in use with more then 10 providers in our team.

Currently the framework has its home in my github account, but it would be subject of contribution to crossplane or crossplane-contrib.

Feel free to give feedback here and directly in the repository.

xp-testing is based e2e-framework in v0.2.0. main_test.go it is setting up a kind cluster, installing crossplane + the respective provider, e.g. pushing xpkg & images into packagecache / kind. Additionally there is a function which installs argocd in the kind cluster.

The tests themselvs work with features and use default create & delete functions & asses the state of the managed resources.

The ci workflow has been extended; the logs of kind will be uploaded as as a build result.

Please do not merge.

I have:

How has this code been tested

negz commented 1 year ago

FWIW there’s some prior art at https://github.com/crossplane/conformance. It's not strictly E2E tests (and 2 years stale) but it's similar.

+1 for using e2e-framework - have you seen https://github.com/crossplane/crossplane/tree/master/test/e2e? If we're going to establish a pattern for e2e-testing of providers I'd like to have it match what we do in c/c.

That said, https://github.com/upbound/uptest also exists and intends to solve this problem. How would you compare the two?

mirzakopic commented 1 year ago

Hi @negz, thanks for the comments. Please find our responses below.

FWIW there’s some prior art at https://github.com/crossplane/conformance. It's not strictly E2E tests (and 2 years stale) but it's similar.

We agree that conformance testing should be part of a larger testing strategy. Our journey into this began last year, with the aim of creating a solution that enables us to conduct e2e tests at a more granular level. Ultimately, our team developed a compact library that empowers us to execute e2e tests within PRs specifically for managed resources from various providers. We encourage further discussions on this topic.

+1 for using e2e-framework - have you seen https://github.com/crossplane/crossplane/tree/master/test/e2e? If we're going to establish a pattern for e2e-testing of providers I'd like to have it match what we do in c/c.

Yes, we're thrilled to see this development, especially considering the coincidence that it aligns with the same framework we chose for this library! 😊

We believe it's a fantastic way to incorporate that for the core components. Over the past year, we've been actively utilizing the e2e-framework, primarily focusing on provider-specific features and releases to uphold a high standard of quality.

Regarding e2e testing in c/c, we concur that it should align with what you've already initiated in c/c. As for the code's repository, we don't have a strong preference for its exact location. We're eager to contribute this to the community, especially after we have had much success with it and shared this in our latest Crossplane meetup in Munich with other contributors and users of Crossplane who are showing great interest in using such a library.

We're open to discussing whether this should remain a separate repository or be integrated into c/c, where both the Crossplane core and providers can benefit from it. We just need to consider potential dependencies and related factors.

That said, https://github.com/upbound/uptest also exists and intends to solve this problem. How would you compare the two?

Regarding Uptest, we don't view it as an exclusive option. In several instances, especially when we needed to assess updates and validations, we found that Kuttle-based tests weren't sufficient for our requirements. This led us to explore the e2e-framework, which, coincidentally, aligns well with the ongoing E2E developments in c/c.

Additionally, we recognize the potential and we could even imagine to enable running of Uptest cases with the spin up clusters from e2e.

Essentially, our perspective aligns with the arguments presented in the e2e test paper.

janwillies commented 10 months ago

Oh, and could you please squash the commits?

maximilianbraun commented 9 months ago

After #110 we should be able to continue here.

github-actions[bot] commented 6 months ago

Crossplane does not currently have enough maintainers to address every issue and pull request. This pull request has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 7 days if no further activity occurs. closed in 7 days if no further activity occurs. Adding a comment starting with /fresh will mark this PR as not stale.

MisterMX commented 6 months ago

/fresh

negz commented 5 months ago

Regarding Uptest, we don't view it as an exclusive option. In several instances, especially when we needed to assess updates and validations, we found that Kuttle-based tests weren't sufficient for our requirements. This led us to explore the e2e-framework, which, coincidentally, aligns well with the ongoing E2E developments in c/c.

Perhaps it would be valuable to try to converge on a single E2E test approach for providers? Or if not, make sure we understand when to use uptest vs when to use e2e-framework? I think we used to have a SIG for E2E testing that we could resurrect for this.

CC @sergenyalcin and @ulucinar, mostly to make sure you're aware of the efforts here.

github-actions[bot] commented 2 months ago

Crossplane does not currently have enough maintainers to address every issue and pull request. This pull request has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 7 days if no further activity occurs. closed in 7 days if no further activity occurs. Adding a comment starting with /fresh will mark this PR as not stale.