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.

Sign up for GitHub

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

Jump to bottom

Running tests without calling environment.InitFlags() func leads to failed test being skipped instead of failure #268

Open
cardil opened this issue Dec 23, 2021 · 7 comments
Open

Running tests without calling environment.InitFlags() func leads to failed test being skipped instead of failure #268

cardil opened this issue Dec 23, 2021 · 7 comments
Assignees
@cardil
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Issues which should be fixed (post-triage)

Comments

@cardil
Copy link
Contributor

cardil commented Dec 23, 2021

The environment.InitFlags() is required to be executed. Without it, the flags are not set properly. Their values are outside of possible known values - s == 0 and l == 0.

https://github.com/knative-sandbox/reconciler-test/blob/816f2192fec9b6363c2b9ed79875aa54d472eede/pkg/environment/flags.go#L29-L30

This leads to nasty behavior - the shouldFail(s *feature.Step) bool func returns false, and instead of failures, the tests are skipped. This behavior is hard to understand.

https://github.com/knative-sandbox/reconciler-test/blob/816f2192fec9b6363c2b9ed79875aa54d472eede/pkg/environment/magic.go#L306-L309

I think, we should amend the test framework to ensure flags have valid values. Either by forcing people to call environment.InitFlags(), or setting flag values with reasonable defaults.
@cardil
Copy link
Contributor Author

cardil commented Dec 23, 2021

/kind bug

@knative-prow-robot knative-prow-robot added the kind/bug Categorizes issue or PR as related to a bug. label Dec 23, 2021
@cardil cardil mentioned this issue Dec 23, 2021
Closed
cardil added a commit to cardil/kn-plugin-event that referenced this issue Dec 23, 2021
@cardil
Make sure to call environment.InitFlags func
9e72570 
See also: knative-extensions/reconciler-test#268
knative-prow-robot pushed a commit to knative-extensions/kn-plugin-event that referenced this issue Feb 4, 2022
@cardil
🎁 E2E tests (#126)
5bdeae6 
* Adding `presubmit-tests.sh` script

* Adding codecov

* E2E tests beginning

* Running integration tests from e2e runner

* Eventshub starting, sort of.

* E2E test is working

* Resolver for kn-plugin-event binary location

* Debugging the send error

* Using proper image

* Further dumping of events

* Make sure to call `environment.InitFlags` func

See also: knative-extensions/reconciler-test#268

* Upgrade knative.dev/reconciler-test

* Clean up the e2e test

* Test for sending to Knative service

* Use library logger

* Auto skip if short flag is used.
cardil added a commit to cardil/kn-plugin-event that referenced this issue Feb 21, 2022
@cardil
🎁 E2E tests (knative-extensions#126)
fc7bfb5 
* Adding `presubmit-tests.sh` script

* Adding codecov

* E2E tests beginning

* Running integration tests from e2e runner

* Eventshub starting, sort of.

* E2E test is working

* Resolver for kn-plugin-event binary location

* Debugging the send error

* Using proper image

* Further dumping of events

* Make sure to call `environment.InitFlags` func

See also: knative-extensions/reconciler-test#268

* Upgrade knative.dev/reconciler-test

* Clean up the e2e test

* Test for sending to Knative service

* Use library logger

* Auto skip if short flag is used.
cardil added a commit to cardil/kn-plugin-event that referenced this issue Feb 21, 2022
@cardil
🎁 E2E tests (knative-extensions#126)
f9b5357 
* Adding `presubmit-tests.sh` script

* Adding codecov

* E2E tests beginning

* Running integration tests from e2e runner

* Eventshub starting, sort of.

* E2E test is working

* Resolver for kn-plugin-event binary location

* Debugging the send error

* Using proper image

* Further dumping of events

* Make sure to call `environment.InitFlags` func

See also: knative-extensions/reconciler-test#268

* Upgrade knative.dev/reconciler-test

* Clean up the e2e test

* Test for sending to Knative service

* Use library logger

* Auto skip if short flag is used.
openshift-merge-robot pushed a commit to openshift-knative/kn-plugin-event that referenced this issue Feb 23, 2022
@cardil
[1.0] Backport e2e tests and fix for knative-sandbox#145 (#85)
af13ecf 
* 🎁 E2E tests (#126)

* Adding `presubmit-tests.sh` script

* Adding codecov

* E2E tests beginning

* Running integration tests from e2e runner

* Eventshub starting, sort of.

* E2E test is working

* Resolver for kn-plugin-event binary location

* Debugging the send error

* Using proper image

* Further dumping of events

* Make sure to call `environment.InitFlags` func

See also: knative-extensions/reconciler-test#268

* Upgrade knative.dev/reconciler-test

* Clean up the e2e test

* Test for sending to Knative service

* Use library logger

* Auto skip if short flag is used.

* 🧹 Port parts of the eventshub readiness check (knative-extensions/reconciler-test#284)

* 🐛 Always send to cluster local Ksvc (#146)

* 🎁 Tests for Eventing (Broker & Channel) (#159)

* Tests for Broker

* Test for Channel

* Enhance logging

* Use the same version of golangci-lint as used on upstream.
@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 24, 2022
@cardil
Copy link
Contributor Author

cardil commented Apr 26, 2022

/reopen
/remove-lifecycle stale

@knative-prow knative-prow bot reopened this Apr 26, 2022
@knative-prow
Copy link

knative-prow bot commented Apr 26, 2022

@cardil: Reopened this issue.

In response to this:

/reopen
/remove-lifecycle stale

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@knative-prow knative-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 26, 2022
@cardil
Copy link
Contributor Author

cardil commented Jul 6, 2022

/assign

@github-actions
Copy link

github-actions bot commented Oct 5, 2022

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 5, 2022
@cardil
Copy link
Contributor Author

cardil commented Oct 5, 2022

/remove-lifecycle stale
/triage accepted

@knative-prow knative-prow bot added triage/accepted Issues which should be fixed (post-triage) and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cardil cardil

Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Issues which should be fixed (post-triage)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

🐛 Ensure the environment.InitFlags() is called to avoid confusion. cardil/knative-reconciler-test
2 participants
@cardil @knative-prow-robot