Skip to content
This repository was archived by the owner on Sep 17, 2024. It is now read-only.

Use "go test" as test runner#104

Merged
mdelapenya merged 8 commits intoelastic:masterfrom
mdelapenya:use-go-test-as-runner
Apr 16, 2020
Merged

Use "go test" as test runner#104
mdelapenya merged 8 commits intoelastic:masterfrom
mdelapenya:use-go-test-as-runner

Conversation

@mdelapenya
Copy link
Copy Markdown
Contributor

@mdelapenya mdelapenya commented Apr 9, 2020

What is this PR doing?

It uses go test instead of the godog binary, so we are removing its installation in the Makefile.

We also remove all gherkin tags supporting godog's filters, creating a configuration layer (a map) with the supported modules. This is needed because at this moment godog is executing all life cycle methods (befores and afters), even when filtering by tag.

We are replacing the usage of tags with the arguments passed to the go test command:

$ go test -v --godog.format pretty redis mysql apache

Why is it important?

With this configuration we are able to define which FeatureContext functions are contributed to the test runner, so inspecting the arguments we will be able to check if it's a supported module. This way, only those contributors of interest will be added to the test suite, only executing the life cycle methods of interest.

Related issues

I created a sample repo demonstrating this life cycle issue: http://github.com/mdelapenya/sample-godog

Because we wan to use the test runner, we are creating a configuration layer
to bypass the existing issue with godog, which is running all life cycle
test methods (before and afters) (See https://github.com/mdelapenya/sample-godog)
For that reason, meanwhile the upstream project fixes this, we have to
maintain that configuration map.
@mdelapenya
Copy link
Copy Markdown
Contributor Author

mdelapenya commented Apr 9, 2020

The existing CI failure https://apm-ci.elastic.co/blue/organizations/jenkins/beats%2Fpoc-metricbeat-tests-poc-mbp/detail/PR-104/1/pipeline/196 happens because go test is printing testing: warning: no tests to run after test execution and our junit parser is not able to generate the XML properly.

I read the Go test code, and this message happens only here:

@jsoriano any idea why it happens?

@mdelapenya mdelapenya requested review from a team and jsoriano April 9, 2020 14:57
@mdelapenya mdelapenya self-assigned this Apr 9, 2020
@mdelapenya
Copy link
Copy Markdown
Contributor Author

I saw the problem: the grep parser is removing the last line, which closes the testsuites element (</testsuites>), as this line is continued with the testing: warning: no tests to run

Comment thread .ci/scripts/functional-test.sh Outdated
@mdelapenya
Copy link
Copy Markdown
Contributor Author

mdelapenya commented Apr 10, 2020

With this approach, we are removing 4-5 minutes to each branch in the parallel tests, as the kind cluster is not provisioned if not needed.

Comment thread e2e/metricbeat-poc.md Outdated
Comment thread e2e/runner_test.go
return paths
}

var supportedProducts = map[string]*contextMetadata{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this structure can grow a lot, and decouple test from categories, it is odd for me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, same for me :( I think this is a workaround until I find how to fix it in the upstream project (as explained here https://github.com/mdelapenya/sample-godog)

@mdelapenya mdelapenya mentioned this pull request Apr 15, 2020
@mdelapenya
Copy link
Copy Markdown
Contributor Author

We are going to merge this PR but knowing that it's a bit workaround that adds complexity and maintainability concerns: the map linking test modules and the FeatureContext functions could grow A LOT.

This is a design decision that we made to bypass the issue that we currently see in Godog, where all the FeatureContext functions are dynamically exported when running godog (it builds a test binary on the fly), so every test Suite containing life cycle hook (Befores and Afters) will contribute those methods to every execution. In our particular case, the Helm charts test suite starts a Kubernetes cluster using Kind, which is very expensive in time and resources, and we do not want to run that code for any other test suite, possibly causing a performance degradation in the build agent running any other test suite making it fail.

For this reason, after talking and agreeing with @jsoriano, we want to make progress on this project, but having this in mind for future reference.

@mdelapenya mdelapenya merged commit 9348b71 into elastic:master Apr 16, 2020
@mdelapenya mdelapenya deleted the use-go-test-as-runner branch April 16, 2020 18:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants