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

Makefile: Explicitly specify the --output-dir ginkgo CLI flag #2821

Merged

Conversation

timflannagan
Copy link
Contributor

Signed-off-by: timflannagan timflannagan@gmail.com

Description of the change:

Explicitly specify the --output-dir ginkgo v2 CLI flag when the ARTIFACT_DIR has been set in CI environments.

Motivation for the change:

Ensure junit reports are being correctly generated when 1.) the top-level directory in the ARTIFACT_DIR doesn't exist, and ginkgo v2 will created it for you 2.) the absolute path to the ARTIFACT_DIR already exists, but ginkgo v2 appends the nested directory structure. This seems to smooth out this experience, and we have the added benefit where we don't need to think about whether the artifact directory variable has been properly set during $(ARTIFACT_DIR)junit_e2e.xml.

Architectural changes:

Testing remarks:

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Bug fixes are accompanied by regression test(s)
  • e2e tests and flake fixes are accompanied evidence of flake testing, e.g. executing the test 100(0) times
  • tech debt/todo is accompanied by issue link(s) in comments in the surrounding code
  • Tests are comprehensible, e.g. Ginkgo DSL is being used appropriately
  • Docs updated or added to /doc
  • Commit messages sensible and descriptive
  • Tests marked as [FLAKE] are truly flaky and have an issue
  • Code is properly formatted

Signed-off-by: timflannagan <timflannagan@gmail.com>
@openshift-ci
Copy link

openshift-ci bot commented Jul 27, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: timflannagan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 27, 2022
@timflannagan
Copy link
Contributor Author

I caught this in downstream CI environments:

  Failed to generate JUnit report:
  open /go/src/github.com/openshift/operator-framework-olm/staging/operator-lifecycle-manager/test/e2e/logs/artifacts/junit_e2e.xml: no such file or directory
  In [ReportAfterSuite] at: /go/src/github.com/openshift/operator-framework-olm/staging/operator-lifecycle-manager/vendor/github.com/onsi/ginkgo/v2/reporting_dsl.go:127

Despite CI properly exposing that ARTIFACT_DIR variable, it looks like we need to always specify the --output-directory CLI flag to make sure this works there as well.

@perdasilva
Copy link
Collaborator

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 27, 2022
@timflannagan
Copy link
Contributor Author

Just want to double check the artifacts directory is still being populated before automation merges this.

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 27, 2022
@timflannagan
Copy link
Contributor Author

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 27, 2022
@timflannagan timflannagan merged commit 0fa6f29 into operator-framework:master Jul 27, 2022
@timflannagan timflannagan deleted the update-ginkgo-v2-options branch July 27, 2022 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants