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 e2e metrics test clean itself up #6742

Closed
tgross opened this issue Nov 20, 2019 · 4 comments · Fixed by #6842
Closed

make e2e metrics test clean itself up #6742

tgross opened this issue Nov 20, 2019 · 4 comments · Fixed by #6842
Assignees
Milestone

Comments

@tgross
Copy link
Member

tgross commented Nov 20, 2019

The e2e tests for metrics don't get run regularly because they have side-effects and don't clean themselves up. This issue is to update the tests so that they can be run by default as part of the nightly e2e testing.

var metrics = flag.Bool("metrics", false, "run metrics tests")
...

// TestMetrics runs fabio/prometheus and waits for those to succeed
// After that a series of jobs added to the input directory are executed
// Unlike other e2e tests this test does not clean up after itself.
// This test is meant for AWS environments and will not work locally
func TestMetrics(t *testing.T) {
	if !*metrics {
		t.Skip("skipping test in non-metrics mode.")
	}
@tgross tgross added this to the near-term milestone Nov 20, 2019
@tgross tgross self-assigned this Nov 20, 2019
@tgross
Copy link
Member Author

tgross commented Nov 25, 2019

Might want to look at #6771 for an example of what could be done to reduce test cross-contamination in metrics.

@notnoop
Copy link
Contributor

notnoop commented Nov 25, 2019

We can probably get more isolation by setting a new global metrics sink through metrics.NewGlobal . We can do it for every tests, or just for the relevant tests with assertions on metrics, as it impacts allowed test parallelism.

It seems we are stuck with using the global metrics sink. Few vendored libraries (e.g. raft, serf, memberlist) use the global metrics sink and changing them complicates their apis and too burdensome just for the benefit of our testing story imo.

@tgross tgross modified the milestones: near-term, 0.10.3 Dec 9, 2019
@tgross
Copy link
Member Author

tgross commented Dec 11, 2019

When I dug into this, I discovered it wasn't a question of polluting the metrics contents in E2E, but that the test had never been wired up into the E2E framework so there was no cleanup of the jobs when we were done. Done in #6842

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants