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

add initial e2e tests #239

Merged
merged 1 commit into from
May 2, 2024
Merged

Conversation

nabuskey
Copy link
Collaborator

@nabuskey nabuskey commented May 1, 2024

This adds initial e2e tests for core packages only.

We are mainly testing rest API responses so we have flexibility in testing tools to use.

Use a scripting languages like Python or JavaScript

  • They are easier and less verbose to write. Probably easier for people not familiar with Go to contribute too.
  • Rich selection of tools from mature ecosystems.

I think this is one of common approaches. What I do not like about this is the mental overhead when switching to another language. I'm not too good at switching from one language to another when writing things.
Bringing another language means we are bloating the requirements to work on this project. In addition to installing language runtime, we will need to introduce setup scripts and guides for the chosen language. For example, installing the right version of python then setting up venv.

I felt that the potential repository bloat and overhead were not worth it. So I stuck with Go.

Use a testing packages for Go

I thought of using frameworks like https://github.com/gavv/httpexpect. I ultimately chose not to use this because I didn't want to introduce another dependency, but I am open to using something like this as long as we reach a consensus.

Tests for examples in the examples direcotry

We should introduce tests for examples like the ref implementation as well, but they shouldn't be part of this repository. We should move them to another repository then test against releases of idpbuilder. Testing methods in the repo can be independent of what we choose here.

Signed-off-by: Manabu McCloskey <[email protected]>
Copy link
Contributor

@blakeromano blakeromano left a comment

Choose a reason for hiding this comment

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

I overall think this is a great start. I wonder if we would have any value in combining this with a tool like chainsaw https://kyverno.github.io/chainsaw/latest/ to do things like make assertions around manifests deployed in the cluster? For example we could make assertions that specific resources have been deployed into the cluster and have particular statuses and use that rather then actually trying to validate that ports are open and making API Calls to ArgoCD?

@greghaynes
Copy link
Contributor

I love this! thanks for adding it.

Theres also this: https://github.com/kubernetes-sigs/e2e-framework - I've never used it so not sure if its actually worth it but just FYI.

@cmoulliard
Copy link
Contributor

+1 to use go as the testing language as this is also the language of this project ;-)
As go testing framework, using an assertion framework will help: https://github.com/stretchr/testify

@nabuskey
Copy link
Collaborator Author

nabuskey commented May 2, 2024

I've looked at k8s e2e and chainsaw too. They are nice for Kubernetes testing but what I think we should be testing first is the functionality of things deployed by idpbuilder like ArgoCD and Gitea services. Especially accessing these resources from a user's computer. What we deploy is based on official releases so I do expect them to work as far as Kubernetes resources are considered.

I am not saying we shouldn't use these e2e testing tools. I think they are valuable and will be useful as we continue to add more tests and assurances. All I am saying is we should focus on testing the functionality of idpbuilder itself first.

I am using testify to assert already :)

@csantanapr
Copy link
Contributor

csantanapr commented May 2, 2024

I think for idpbuilder scope of function sticking with using go and making so with mininmal external dependency is the way to do, also idpbuilder should not be in charge of testing what's get's deploy to kubernetes other than the core packages which is not that many (argocd, gitea, nginx) so bringing a framework like chainsaw or e2e-framework I would say is too much.

For the examples which we are going to move to another repo, there I can see we use something like e2e-framework or chainsaw to test something like the CNOE ref-implementation. In the new repo we can even support both that the maintainer of an example folder wants to use, they would add a script (ie. e2e.sh) that get's call on every folder in examples/stacks provided from github actions to run e2e using the latest version of idpbuilder this way we know that the examples are always healthy with the latest version of idpbuilder

@nimakaviani nimakaviani self-requested a review May 2, 2024 18:59
Copy link
Contributor

@nimakaviani nimakaviani left a comment

Choose a reason for hiding this comment

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

+1 to what Manabu and Carlos said on keeping the tests minimal and focused on user functionality.

LGTM

@nimakaviani nimakaviani merged commit 1c40bbd into cnoe-io:main May 2, 2024
2 checks passed
@nabuskey nabuskey deleted the initial-e2e-tests branch May 2, 2024 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants