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

Refactor e2e test by using some common functions #100

Closed
zhangtbj opened this issue Apr 9, 2020 · 6 comments
Closed

Refactor e2e test by using some common functions #100

zhangtbj opened this issue Apr 9, 2020 · 6 comments

Comments

@zhangtbj
Copy link
Contributor

zhangtbj commented Apr 9, 2020

1, In the current e2e test framework, we run each test one by one and most of the code can be extracted as a common function:
https://github.com/redhat-developer/build/blob/master/test/e2e/main_test.go#L82-L110

The background behind this issue is: Right now, in our tenant environment, we only provide kaniko and buildpacks build strategies (buildah requires privileged permission which is not allowed in our env)

So we require to just run kaniko and buildpacks e2e tests.

So we would like to extract the common test code as a shared function like:

func RunBuildTest(xxx) {
	createClusterBuildStrategy(t, ctx, f, oE.clusterBuildStrategy)
	validateController(t, ctx, f, oE.build, oE.buildRun)
	deleteClusterBuildStrategy(t, f, oE.clusterBuildStrategy)

	// Run e2e tests
	oE = newOperatorEmulation(namespace,
		"example-build-s2i",
		"samples/buildstrategy/xxx/buildstrategy_xxx_cr.yaml",
		"samples/build/build_xxx_cr.yaml",
		"samples/buildrun/buildrun_xxx_cr.yaml",
	)
	err = BuildTestData(oE)
	require.NoError(t, err)
	validateOutputEnvVars(oE.build)

	createClusterBuildStrategy(t, ctx, f, oE.clusterBuildStrategy)
	validateController(t, ctx, f, oE.build, oE.buildRun)
	deleteClusterBuildStrategy(t, f, oE.clusterBuildStrategy)
}

So that we can refine the e2e test code and disable any one easily like:

  • RunBuildTest(kaniko, private_repo)
  • RunBuildTest(buildpacks, private_repo)
  • // RunBuildTest(buildah, private_repo)
  • // RunBuildTest(s2i, private_repo)

And we pre-install the ClusterBuildStrategies on the env first, also we also need to find a way to skip the createClusterBuildStrategy(t, ctx, f, oE.clusterBuildStrategy) step.

2, And we also would like to create a new function to include those schema validation:
https://github.com/redhat-developer/build/blob/master/test/e2e/main_test.go#L30-L53

Let me know if you have any comment. @sbose78 and @qu1queee

@qu1queee
Copy link
Contributor

qu1queee commented Apr 9, 2020

I spoke with @zhangtbj on this, I think being able to specify which strategies to run during e2e tests isn´t ideal, e2e tests should run everything. In the meantime, we can live with a minor patch from our side to disable the two strategies we cannot run successfully(buildah and s2i).

However, I think we would like to have the flexibility to avoid running the following pieces during e2e tests, due to our specific tenant environments and the way where we run components:

  • Avoid running the framework.AddToFrameworkScheme , see lines
  • Avoid running the ctx.InitializeClusterResources, see lines
  • Avoid running the e2eutil.WaitForOperatorDeployment, see lines
  • Avoid running the createClusterBuildStrategy calls for all strategies, see lines
  • Avoid running the tekton task verification , see lines , this is because we cannot check the tekton resource on the tenant namespace.

All of the above can be done with a new env variable, I can do this easily(I think), @sbose78 pls let us know what u think. With this we will only patch to skip those two strategies, and once we resolved the issues, we will not longer need to patch anything. Also, this will not break your setup.

@qu1queee
Copy link
Contributor

@sbose78 let me know about the above.

@sbose78
Copy link
Member

sbose78 commented Apr 20, 2020

If I understand this correctly,

  1. There are specific strategies that we would want skipped.

  2. There are specific things we would not want to validate. Example, tekton resources.

Can we make our tests resilient enough to understand that specific validations may be skipped?

Which user do you run these tests as in your internal environment?

  1. In my opinion we should create "environment templates" in our upsteam CI for running tests in such environments.
  2. Make our tests smarter to skip validating Specific things based on the environment.

@zhangtbj
Copy link
Contributor Author

Agree.

Because our Production system is ready and deployed at the beginning (build controller and cluster build strategies). So every time, we just need to update the controller and re-run the e2e tests.

  • So require to deploy the builder controller and cluster build strategies is not good for our production system upgrade.
  • If I deploy a test env and already deploy above required resources, I would like to run the e2e tests with a similar parameter --skip-deploy to JUST run the e2e tests directly.

@zhangtbj
Copy link
Contributor Author

zhangtbj commented Apr 20, 2020

But ....

It issues is used to extract the current test code as the common functions, so that each test can reuse and share together.

I think later, we can create a new issue to separate these two requirements.

@zhangtbj
Copy link
Contributor Author

This basic requirement is resolved by using this PR:
#120

Close this first and will open the advanced issue for the new requirement.

Thanks!

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

No branches or pull requests

3 participants