Fix flaky tests (1) #5479
Fix flaky tests (1) #5479
Conversation
It has been observed that in rare cases, this regression test has spurious failures in CI. Try increasing the threshold for failure a bit and see if this makes it pass consistently.
|
Lots of CI failures on Azure. Looked at how Azure CI job works, and noted that it runs the test suite 3 times against the same test DB. Noticed that I wasn't cleaning out the renamed test table correctly and so the test would always fail on the 2nd and 3rd test run against the same DB. Just seeing if I can fix that... if it works, will tidy code up a bit and squash commits. |
alexdowad
force-pushed
the
fix-flaky-tests-1
branch
from
aff9d36 to
08d9635
Compare
|
Trying something else. I think it should really kill this problem dead. |
This test creates a MySQL table called 'test'. In several cases, I have seen a spurious test failure (in CI) with an error message saying: "table 'test' already exists". It may be that another test had used a table with the same name and not cleaned it out correctly. Or maybe we have multiple tests running in parallel in some CI environments, or the same test DB being used for multiple runs of the test suite. In any case, change the table name so it is exclusive to this test case only. Also, if the test table exists at the beginning of the test, drop it.
alexdowad
force-pushed
the
fix-flaky-tests-1
branch
from
08d9635 to
2698918
Compare
| * Allow one more to account for a possible process switch. */ | ||
| if ($moreThanLimit > 2) { | ||
| * Allow two more to account for possible process switch, etc. */ | ||
| if ($moreThanLimit > 3) { | ||
| echo "fgets() took more than $max_ms ms $moreThanLimit times\n"; | ||
| } |
There was a problem hiding this comment.
Do you remember which builder this one failed on?
There was a problem hiding this comment.
I've seen this one fail repeatedly. At least once on Travis CI, in the S390X environment, but I think I have seen it in other environments as well.
|
@carusogabriel thanks for the mention. I am still relatively new to the PHP development process. I hope the PR is considered useful. |
|
I agree with @carusogabriel. Your PR is more comprehensive than mine. Looking forward to the merge. |
|
I'll hijack this to collect flaky tests...
|
| @@ -71,10 +71,15 @@ memory_limit=256M | |||
| printf("[011] Failed to change max_allowed_packet"); | |||
| } | |||
|
|
|||
| if (!mysqli_query($link, "CREATE TABLE test(col_blob LONGBLOB) ENGINE=" . $engine)) | |||
| printf("[012] [%d] %s\n", mysqli_errno($link), mysqli_error($link)); | |||
| $table_name = 'test__mysqli_insert_packet_overflow'; | |||
There was a problem hiding this comment.
I am having trouble articulating why I did that. Maybe I wanted 'test' to appear distinct from the name of the test case?
|
@nikic You identified the root cause? What was it? |
|
@alexdowad See #5447 (comment) and following. The fix is 9a98569. |
|
session_module_name_variation4.phpt is a bad one. I have already spent a fair amount of time investigating it, but no results so far. The other bad ones are the lstat tests and the FPM tests. |
|
I think I have seen session_module_name_variation4.phpt failing on AppVeyor occasionally as well. |
|
Maybe we should change all tests creating a equal named table? |
|
@staabm Unless there is interest in running mysql tests in parallel, unique test table names are not necessary. |
|
(and it passed! thanks!) |
I have identified several other tests which fail intermittently, and intend to sort those out as well.