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

Unit tests are broken #1659

Closed
afrittoli opened this issue Dec 2, 2019 · 2 comments · Fixed by #1731
Closed

Unit tests are broken #1659

afrittoli opened this issue Dec 2, 2019 · 2 comments · Fixed by #1731
Assignees
@hrishin
Labels
area/testing Issues or PRs related to testing kind/flake Categorizes issue or PR as related to a flakey test

Comments

@afrittoli
Copy link
Member

Expected Behavior

Unit tests pass in CI and in (nightly) releases.

Actual Behavior

Unit tests pass in CI but fail in releases:

2019-11-27T02:02:43.168Z	ERROR	TestReconcile_InvalidPipelineRuns/invalid-pipeline-run-params-dont-exist-shd-stop-reconciling	pipelinerun/controller.go:61	Failed to create pipelinerun metrics recorder running_pipelineruns_count: cannot register view "running_pipelineruns_count"; a different view with the same name is already registered
github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun.NewController.func1
	/workspace/src/github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/controller.go:61
github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun.getPipelineRunController
	/workspace/src/github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/pipelinerun_test.go:73
github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun.TestReconcile_InvalidPipelineRuns.func1
	/workspace/src/github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/pipelinerun_test.go:373
testing.tRunner
	/usr/local/go/src/testing/testing.go:865

Steps to Reproduce the Problem

  1. Run the unit tests task used by the release pipeline

Additional Info
@afrittoli
Copy link
Member Author

/assign @hrishin

@afrittoli afrittoli assigned afrittoli and hrishin and unassigned hrishin and afrittoli Dec 2, 2019
@vdemeester vdemeester added area/testing Issues or PRs related to testing kind/flake Categorizes issue or PR as related to a flakey test labels Dec 3, 2019
hrishin added a commit to hrishin/tekton-pipeline that referenced this issue Dec 11, 2019
@hrishin
Fixes broken metrics unit tests
5310347 
Quite often unit test failure showing "*** a different view
with the same name is already registered" messages at many places.
At the biggning of every metrics test, it tries to register respective
metrics views(metrics data it want present), publish metrics and
uniregister views. Uniregister view is important becuase for next
test it dont need to hold the old test data.
unregister() views were getting called using `defer` in
table test for loop, just like
for _, td := range tests {
	defer unregister()
	.......
}
This defer actually gets execute after completion of test function rather
than for each loop. This were resulting unwanted execution behaviour
for unregister() views function, which umtimately faling tests.

This patch fixes the behaviour by calling uniregister() views function
at the end of loop.

Fixes tektoncd#1659
@hrishin hrishin mentioned this issue Dec 11, 2019
Merged
3 tasks
@hrishin
Copy link
Member

hrishin commented Dec 11, 2019

This fix hopefully should resolve failing tests for #1709

hrishin added a commit to hrishin/tekton-pipeline that referenced this issue Dec 11, 2019
@hrishin
Fixes broken metrics unit tests
a19543d 
Quite often unit test failure showing "*** a different view
with the same name is already registered" messages at many places.
At the biggning of every metrics test, it tries to register respective
metrics views(metrics data it want present), publish metrics and
uniregister views. Uniregister view is important becuase for next
test it dont need to hold the old metrics data.
unregister() views were getting called using `defer` in
table test for loop, just like
for _, td := range tests {
	defer unregister()
	.......
}
This were resulting unwanted execution behaviour
for unregister() views function, which umtimately faling tests.
Its better to unregister() views before start executing the
tests.

This patch fixes the behaviour by calling uniregister() views function
at the end of loop.

Fixes tektoncd#1659
tekton-robot pushed a commit that referenced this issue Dec 11, 2019
@hrishin @tekton-robot
Fixes broken metrics unit tests
21e0683 
Quite often unit test failure showing "*** a different view
with the same name is already registered" messages at many places.
At the biggning of every metrics test, it tries to register respective
metrics views(metrics data it want present), publish metrics and
uniregister views. Uniregister view is important becuase for next
test it dont need to hold the old metrics data.
unregister() views were getting called using `defer` in
table test for loop, just like
for _, td := range tests {
	defer unregister()
	.......
}
This were resulting unwanted execution behaviour
for unregister() views function, which umtimately faling tests.
Its better to unregister() views before start executing the
tests.

This patch fixes the behaviour by calling uniregister() views function
at the end of loop.

Fixes #1659
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hrishin hrishin

Labels
area/testing Issues or PRs related to testing kind/flake Categorizes issue or PR as related to a flakey test
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes flaky metrics unit tests hrishin/tekton-pipeline
3 participants
@vdemeester @afrittoli @hrishin