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

Move auth and client init into BeforeEach to improve Ginkgo errors #1321

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

mclasmeier
Copy link
Contributor

Description

In case the environment in which to execute e2e tests is missing proper auth tokens, executing the e2e tests can fail as follows:

Assertion or Panic detected during tree construction
var _ = Describe("Fleetshard-sync Targeted Upgrade", func() {
/Users/mclasmeier/go/src/github.com/stackrox/acs-fleet-manager/e2e/e2e_canary_upgrade_test.go:38
  Ginkgo detected a panic while constructing the spec tree.
  You may be trying to make an assertion in the body of a container node
  (i.e. Describe, Context, or When).

  Please ensure all assertions are inside leaf nodes such as BeforeEach,
  It, etc.

  Here's the content of the panic that was caught:
  Your Test Panicked
  Expect(err).ToNot(HaveOccurred())
  /Users/mclasmeier/go/src/github.com/stackrox/acs-fleet-manager/e2e/e2e_canary_upgrade_test.go:52
    When you, or your assertion library, calls Ginkgo's Fail(),
    Ginkgo panics to prevent subsequent assertions from running.

    Normally Ginkgo rescues this panic so you shouldn't see it.

    However, if you make an assertion in a goroutine, Ginkgo can't capture the
    panic.
    To circumvent this, you should call

    	defer GinkgoRecover()

    at the top of the goroutine that caused this panic.

    Alternatively, you may have made an assertion outside of a Ginkgo
    leaf node (e.g. in a container node or some out-of-band function) - please
    move your assertion to
    an appropriate Ginkgo node (e.g. a BeforeSuite, BeforeEach, It, etc...).

    Learn more at:
    http://onsi.github.io/ginkgo/#mental-model-how-ginkgo-handles-failure


  Learn more at: http://onsi.github.io/ginkgo/#no-assertions-in-container-nodes

program not built with -cover
ginkgo run failed
  could not finalize profiles:
  Unable to read coverage file /Users/mclasmeier/go/src/github.com/stackrox/acs-fleet-manager/e2e/cover.profile:
  open /Users/mclasmeier/go/src/github.com/stackrox/acs-fleet-manager/e2e/cover.profile: no such file or directory
You're using deprecated Ginkgo functionality:
=============================================
  --slow-spec-threshold is deprecated --slow-spec-threshold has been deprecated and will be removed in a future version of Ginkgo.  This feature has proved to be more noisy than useful.  You can use --poll-progress-after, instead, to get more actionable feedback about potentially slow specs and understand where they might be getting stuck.

To silence deprecations that can be silenced set the following environment variable:
  ACK_GINKGO_DEPRECATIONS=2.12.0

make: *** [Makefile:348: test/e2e] Error 1

This doesn't point directly to the root cause (missing tokens in environment), instead it tells us to reorganize the test init so that Ginkgo can handle failures gracefully.

This PR follows Ginkgo's advice by moving the potentially panicking code into a BeforeEach construct.

Checklist (Definition of Done)

  • Unit and integration tests added (n/a)
  • ~~Added test description under `Test manual``` (n/a)
  • Documentation added if necessary (i.e. changes to dev setup, test execution, ...) (n/a)
  • CI and all relevant tests are passing
  • Add the ticket number to the PR title if available, i.e. ROX-12345: ... (fix done without ticket)
  • Discussed security and business related topics privately. Will move any security and business related topics that arise to private communication channel. (n/a)
  • Add secret to app-interface Vault or Secrets Manager if necessary (n/a)
  • RDS changes were e2e tested manually (n/a)
  • Check AWS limits are reasonable for changes provisioning new resources (n/a)

Test manual

TODO: Add manual testing efforts

# To run tests locally run:
make db/teardown db/setup db/migrate
make ocm/setup OCM_OFFLINE_TOKEN=<ocm-offline-token> OCM_ENV=development
make verify lint binary test test/integration

@mclasmeier mclasmeier temporarily deployed to development October 9, 2023 09:22 — with GitHub Actions Inactive
@mclasmeier mclasmeier temporarily deployed to development October 9, 2023 09:23 — with GitHub Actions Inactive
@mclasmeier mclasmeier temporarily deployed to development October 9, 2023 09:23 — with GitHub Actions Inactive
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 9, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kurlov, mclasmeier, SimonBaeumer

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:
  • OWNERS [SimonBaeumer,kurlov]

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

@mclasmeier mclasmeier merged commit b812a8a into main Oct 10, 2023
8 checks passed
@mclasmeier mclasmeier deleted the mc/beforeeach-init branch October 10, 2023 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants