Skip to content

Disables openclose_h2 test.#4113

Merged
d2r merged 1 commit intoapache:masterfrom
d2r:disable-opencloseh2
Aug 16, 2018
Merged

Disables openclose_h2 test.#4113
d2r merged 1 commit intoapache:masterfrom
d2r:disable-opencloseh2

Conversation

@d2r
Copy link
Copy Markdown
Contributor

@d2r d2r commented Aug 14, 2018

This test is brittle and repeatedly errors when it runs, yielding false negatives.

This commit permanently skips the test.

@d2r d2r added the Tests label Aug 14, 2018
@d2r d2r added this to the 9.0.0 milestone Aug 14, 2018
@d2r
Copy link
Copy Markdown
Contributor Author

d2r commented Aug 14, 2018

See #3995, for example.

@maskit
Copy link
Copy Markdown
Member

maskit commented Aug 15, 2018

How did you confirm the error is false negative?

I'm not so sure we can just ignore the error because I couldn't reproduce the error on my local machine.

@d2r
Copy link
Copy Markdown
Contributor Author

d2r commented Aug 15, 2018

How did you confirm the error is false negative?

Three jenkins builds #6337 (non-pass), #6338 (non-pass), and #6339 (pass) were run in succession on a branch that did not change between runs.

If it was a true negative, then it should not have passed on the third try.

Otherwise, if it was a false negative, then the test should be fixed so that it can correctly test the functionality.

Looking more at the test, it seems we might be testing that we win a race condition. The comparison that sometimes tests false seems to be a direct equality of two pairs metrics that depend on the execution of client requests in 25 separate threads—but there is not any coordination between them. We wait 10 seconds before checking the metric and hope that there is no race condition by that time.

In the meantime, this would disable the test so that others do not have the same difficulty. We can create an issue to look into improving the test itself. But if we would rather not disable the test then I can close this PR.

This test is brittle and repeatedly errors when it runs, yielding false negatives.

This commit permanently skips the test.
@d2r d2r force-pushed the disable-opencloseh2 branch from 8dcebf8 to 6537a33 Compare August 15, 2018 16:25
@maskit
Copy link
Copy Markdown
Member

maskit commented Aug 16, 2018

Yeah, it could be, but not necessarily. Some transactions might have failed on some conditions. Since this test doesn't check error logs, we don't know what really happened. I thought you were able to confirm it.

I'm OK with disabling this test, but if we don't remove it, we should file an issue to improve it.

@d2r
Copy link
Copy Markdown
Contributor Author

d2r commented Aug 16, 2018

Yeah, it could be, but not necessarily. Some transactions might have failed on some conditions.

This is true. We cannot always tell if it is a true or false negative. I think the central problem here is that it does not execute consistently.

Created #4120 to improve the test.

@d2r d2r merged commit ff074f2 into apache:master Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants