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

adapt esreporter to ginkgo v2 report type #5504

Merged
merged 4 commits into from Mar 8, 2022

Conversation

hendrikKahl
Copy link
Contributor

@hendrikKahl hendrikKahl commented Mar 2, 2022

How to categorize this PR?

/area testing
/kind cleanup

What this PR does / why we need it:
Following the switch to ginkgo v2, the custom reporter still implemented DeprecatedReporter and was invoked via ReportViaDeprecatedReporter . The PR drops the usage of deprecated structures and interfaces and implements a custom reporting using the new Report type only.

Which issue(s) this PR fixes:
Fixes #5321

Special notes for your reviewer:
/invite @dguendisch @timebertt @rfranzke

I'm not entirely sure regarding the categorization of the release note. @timebertt would be great, if you could double-check. Thanks!

Release note:

Use ginkgo v2 `Report` structures and drop usage of deprecated custom reporter. To adapt, replace the call of `reporters.ReportViaDeprecatedReporter` within any `ReportAfterSuite` node with `reporter.ReportResults(*reportFilePath, *esIndex, report)`

Testing
Since there are no unit/integration tests and the expected output structure contains timestamps / durations, I did a smal manual testing to qualify things.

Reports generated by testruns are attached to this PR, but here are also snippets to compare the relevant structure.

using the new reporter - succeeded step

{ "index": { "_index": "gardener-testsuite", "_type": "_doc" } }
{"suite":{"name":"Shoot Test Suite","phase":"Failed","tests":9,"failures":8,"errors":0,"duration":367.428},"name":"[DEFAULT] [RELEASE] [SHOOT] should download shoot kubeconfig successfully","shortName":"[DEFAULT] [RELEASE] [SHOOT] should download shoot kubeconfig successfully","phase":"Succeeded","duration":0.080158809}

using the deprecated reporter - succeeded step

{ "index": { "_index": "gardener-testsuite", "_type": "_doc" } }
{"suite":{"name":"Shoot Test Suite","phase":"Failed","tests":9,"failures":8,"errors":0,"duration":366.86},"name":"[DEFAULT] [RELEASE] [SHOOT] should download shoot kubeconfig successfully","shortName":"[DEFAULT] [RELEASE] [SHOOT] should download shoot kubeconfig successfully","phase":"Succeeded","duration":0.069546686}

using the new reporter - failed step

{"index": { "_index": "gardener-testsuite", "_type": "_doc" } }
{"suite":{"name":"Shoot Test Suite","phase":"Failed","tests":9,"failures":8,"errors":0,"duration":367.428},"name":" [DEFAULT] [SHOOT] should read runtime metrics","shortName":"[DEFAULT] [SHOOT] should read runtime metrics","phase":"Failed","failure":{"type":"Failed","message":"/go/src/github.com/gardener/gardener/test/framework/gingko_utils.go:26\nUnexpected error:\n    \u003c*errors.errorString | 0xc000b8a1a0\u003e: {\n        s: \"could not find template in \\\"/go/src/github.com/gardener/gardener/test/testmachinery/framework/resources/templates/simple-load-deployment.yaml.tpl\\\"\",\n    }\n    could not find template in \"/go/src/github.com/gardener/gardener/test/testmachinery/framework/resources/templates/simple-load-deployment.yaml.tpl\"\noccurred\n/go/src/github.com/gardener/gardener/test/testmachinery/shoots/applications/metrics.go:71"},"duration":10.507470436,"system-out":"[BeforeEach] Shoot application metrics testing..."}

using the deprecated reporter - failed step

{ "index": { "_index": "gardener-testsuite", "_type": "_doc" } }
{"suite":{"name":"Shoot Test Suite","phase":"Failed","tests":9,"failures":8,"errors":0,"duration":366.86},"name":" [DEFAULT] [SHOOT] should read runtime metrics","shortName":"[DEFAULT] [SHOOT] should read runtime metrics","phase":"Failed","failure":{"type":"Failed","message":"/go/src/github.com/gardener/gardener/test/framework/gingko_utils.go:26\nUnexpected error:\n    \u003c*errors.errorString | 0xc000be2f20\u003e: {\n        s: \"could not find template in \\\"/go/src/github.com/gardener/gardener/test/testmachinery/framework/resources/templates/simple-load-deployment.yaml.tpl\\\"\",\n    }\n    could not find template in \"/go/src/github.com/gardener/gardener/test/testmachinery/framework/resources/templates/simple-load-deployment.yaml.tpl\"\noccurred\n/go/src/github.com/gardener/gardener/test/testmachinery/shoots/applications/metrics.go:71"},"duration":10.346779425,"system-out":"[BeforeEach] Shoot application metrics testing..."}

deprecated_report.txt
new_report.txt

@hendrikKahl hendrikKahl requested review from a team as code owners March 2, 2022 15:14
@gardener-robot gardener-robot added area/testing Testing related kind/cleanup Something that is not needed anymore and can be cleaned up size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 2, 2022
@rfranzke
Copy link
Member

rfranzke commented Mar 2, 2022

/invite @acumino

@dguendisch
Copy link
Member

Does it make sense to add a unit tests specifically for this reporting (like what you did manually)? Would potentially give us more confidence in future ginkgo updates.

Copy link
Member

@acumino acumino left a comment

Choose a reason for hiding this comment

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

Thanks

/lgtm

test/framework/reporter/esreporter.go Outdated Show resolved Hide resolved
test/framework/reporter/esreporter.go Show resolved Hide resolved
@hendrikKahl
Copy link
Contributor Author

hendrikKahl commented Mar 4, 2022

@dguendisch @acumino thanks for your suggestions.

I moved the processing of a ginkgo report to a separate function and added some basic tests for it. This should give us some confidence going forward while avoiding to parse & validate the json dump that is produced for the upload to an elasticsearch instance.

@gardener-robot gardener-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs/second-opinion and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. reviewed/lgtm labels Mar 4, 2022
@gardener-robot
Copy link

@rfranzke You have pull request review open invite, please check

Copy link
Member

@dguendisch dguendisch left a comment

Choose a reason for hiding this comment

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

/lgtm

dguendisch
dguendisch previously approved these changes Mar 7, 2022
@rfranzke rfranzke merged commit 708d86e into gardener:master Mar 8, 2022
@hendrikKahl hendrikKahl deleted the tm-reporter branch March 8, 2022 06:55
@timebertt
Copy link
Member

Thanks @hendrikKahl!

krgostev pushed a commit to krgostev/gardener that referenced this pull request Apr 21, 2022
* adapt esreporter to ginkgo v2 report type

* add tests

* report errors and failures

* add license header
krgostev pushed a commit to krgostev/gardener that referenced this pull request Jul 5, 2022
* adapt esreporter to ginkgo v2 report type

* add tests

* report errors and failures

* add license header
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Testing related kind/cleanup Something that is not needed anymore and can be cleaned up size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adapt gardener test framework reporter to ginkgo's new reporting infrastructure
9 participants