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

Make it possible to opt out of junit #950

Merged
merged 7 commits into from
Oct 16, 2019
Merged

Make it possible to opt out of junit #950

merged 7 commits into from
Oct 16, 2019

Conversation

alenkacz
Copy link
Contributor

What this PR does / why we need it:

Fixes #946

Copy link
Member

@nfnt nfnt left a comment

Choose a reason for hiding this comment

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

Looks great, very useful addition. Some small comments from my side, concerning running this script without any set env vars.

@@ -21,7 +21,7 @@ if ! docker inspect "$builder_image"; then
fi

if docker build -f test/Dockerfile -t kudo-test .; then
if docker run -it -m 4g -v "$(pwd)"/reports:/go/src/github.com/kudobuilder/kudo/reports --rm kudo-test; then
if docker run -it -m 4g -v -e INTEGRATION_OUTPUT_JUNIT="$(INTEGRATION_OUTPUT_JUNIT)" "$(pwd)"/reports:/go/src/github.com/kudobuilder/kudo/reports --rm kudo-test; then
Copy link
Member

Choose a reason for hiding this comment

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

Because of set -o nounset above, this script will fail to run if INTEGRATION_OUTPUT_JUNIT isn't set. How about adding a default value that can be overridden from outside to the script?
E.g.

INTEGRATION_OUTPUT_JUNIT=${INTEGRATION_OUTPUT_JUNIT:-false}

else
go test -tags integration ./pkg/... ./cmd/... -v -mod=readonly -coverprofile cover-integration.out
go run ./cmd/kubectl-kudo test
fi
Copy link
Member

Choose a reason for hiding this comment

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

Nitpicking: Please add a newline at the end of files.

@alenkacz
Copy link
Contributor Author

@nfnt all should be addressed

Copy link
Member

@nfnt nfnt left a comment

Choose a reason for hiding this comment

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

LGTM!

@gerred
Copy link
Member

gerred commented Oct 15, 2019

are we shellchecking these? that might make a nice github action. LGTM.

@alenkacz alenkacz merged commit 43faa62 into master Oct 16, 2019
@alenkacz alenkacz deleted the av/junit-opt-out branch October 16, 2019 07:08
jbarrick-mesosphere pushed a commit that referenced this pull request Oct 17, 2019
* Introduce integration test for restart (#718)

This introduces and integration test that:
- creates instance
- waits for deploy on that instance is finished
- stops controller
- updates instance CRD in the meantime
- starts controller
- verifies update plan was run

This test was failing prior to the instance controller refactoring (the old controller logic was handling this wrong)

* Fix namespace handling for plan command (#925)

* Include CRDs in the -o yaml --dry-run (#923)

* go mod changes makes repo dirty.  this resolves that (#926)

* Fix deep copy and don't modify original state (#919)

* Fixes the release workflow for Docker images

* Attempt to fix 'TestCheckResourceIntegration' flaky test (#914)

* Do not use strategicmergepatch for CRDs (#936)

* Plan status should show last executed plan, not just active plan (#931)

* More consistent logging for install (#933)

* fixing repo merge by name bug (#938)

* Use built in isExists (#942)

* Make it possible to opt out of junit (#950)

* Maintainers is array, not object (#930)

* Handle already exists in flaky test (#941)

* Cleanup lastexecutedplan method to remove the flake (#949)

* Fix docs for instance name in install (#943)

* add namespace flag for kudo init (#903)

* Fix 'run_tests' script

* Use a shell script for E2E test commands

The script is similar to the one used for integration tests. This also
allows to not use JUnit output.

* Copy E2E test script into test container

* Fix permissions of E2E test script

* Distinguish between local and operator E2E tests

Tests in the 'test/e2e' folder are run as part of the E2E tests in addition
to the operator tests.
jbarrick-mesosphere added a commit that referenced this pull request Oct 17, 2019
fix make

docker privs

add go-junit-report

add docker CLI to test dockerfile

fix docker cli install

fix docker run command

fix docker

fix networking

remove CMD from dockerfile

E2E test additions (#958)

* Introduce integration test for restart (#718)

This introduces and integration test that:
- creates instance
- waits for deploy on that instance is finished
- stops controller
- updates instance CRD in the meantime
- starts controller
- verifies update plan was run

This test was failing prior to the instance controller refactoring (the old controller logic was handling this wrong)

* Fix namespace handling for plan command (#925)

* Include CRDs in the -o yaml --dry-run (#923)

* go mod changes makes repo dirty.  this resolves that (#926)

* Fix deep copy and don't modify original state (#919)

* Fixes the release workflow for Docker images

* Attempt to fix 'TestCheckResourceIntegration' flaky test (#914)

* Do not use strategicmergepatch for CRDs (#936)

* Plan status should show last executed plan, not just active plan (#931)

* More consistent logging for install (#933)

* fixing repo merge by name bug (#938)

* Use built in isExists (#942)

* Make it possible to opt out of junit (#950)

* Maintainers is array, not object (#930)

* Handle already exists in flaky test (#941)

* Cleanup lastexecutedplan method to remove the flake (#949)

* Fix docs for instance name in install (#943)

* add namespace flag for kudo init (#903)

* Fix 'run_tests' script

* Use a shell script for E2E test commands

The script is similar to the one used for integration tests. This also
allows to not use JUnit output.

* Copy E2E test script into test container

* Fix permissions of E2E test script

* Distinguish between local and operator E2E tests

Tests in the 'test/e2e' folder are run as part of the E2E tests in addition
to the operator tests.
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.

Integration-test doesn't work offline
3 participants