Test stability umbrella bug #117

jonahgraham · 2022-10-25T15:11:45Z

This is a list of known unstable tests since we moved to Java 17 + GitHub Actions.

If you have a failing test in your GitHub actions, please check it against this list. If the test looks unstable, add it to this list (or make a comment requesting a committer do that).

It may be that this test fails regularly because another test is not cleaned up properly. Make sure there are no unexpected projects in the workspace. Part of eclipse-cdt#117

Part of eclipse-cdt#117

Various tests are not cleaning up properly after themselves, causing test failures on subsequent tests. Therefore start each test by deleting all projects. This required a bit of restructuring as some tests relied on this behaviour too. Part of eclipse-cdt#117

This old test had a race condition. The failing test was trying to verify that CDTPROJECT_ADDED was received, but if there was a delay in the startup then another event would come in later. So for this test use the first received event, for the remaining tests use the last received event. Part of eclipse-cdt#117

On GitHub actions the org.eclipse.cdt.ui.tests.text.contentassist2 tests are running after ProposalFilterPreferencesTest and ProposalFilterPreferencesTest was changing the default filter and not restoring it. Part of eclipse-cdt#117

jonahgraham · 2022-10-25T17:28:54Z

So it looks like most of the current failures we are seeing is because of changes in test order due to a combination of Java 17 + GitHub actions that may affect test order (which is not generally deterministic). See #118 for fixes to many of the test failures already.

This old test had a race condition. The failing test was trying to verify that CDTPROJECT_ADDED was received, but if there was a delay in the startup then another event would come in later. So for this test use the first received event, for the remaining tests use the last received event. Part of eclipse-cdt#117

On GitHub actions the org.eclipse.cdt.ui.tests.text.contentassist2 tests are running after ProposalFilterPreferencesTest and ProposalFilterPreferencesTest was changing the default filter and not restoring it. Part of eclipse-cdt#117

Various tests are not cleaning up properly after themselves, causing test failures on subsequent tests. Therefore start each test by deleting all projects. For older JUnit3 style tests: This substantially slows down tests as many tests rely on sharing the project between multiple tests and recreating those projects on each run is slow. Therefore this is not applied universally to all JUnit3 tests. For tests that are affected, those tests are moved to JUnit5 base test. Part of eclipse-cdt#117

Part of eclipse-cdt#117

Various tests are not cleaning up properly after themselves, causing test failures on subsequent tests. Therefore start each test by deleting all projects. For older JUnit3 style tests: This substantially slows down tests as many tests rely on sharing the project between multiple tests and recreating those projects on each run is slow. Therefore this is not applied universally to all JUnit3 tests. For tests that are affected, those tests are moved to JUnit5 base test. Part of eclipse-cdt#117

@AfterEach

Various tests are not cleaning up properly after themselves, causing test failures on subsequent tests. Therefore start each test by deleting all projects. In addition, some tests were creating their test projects in their constructor. As all the constructors run before all the tests as part of test discovery it means that projects were being created in constructor and interfering with other tests later. With the deleting of all projects in @AfterEach these tests would have started failing. Therefore, change these tests to create their projects and do other initialize tasks in the setUp method. For older JUnit3 style tests: This substantially slows down tests as many tests rely on sharing the project between multiple tests and recreating those projects on each run is slow. Therefore this is not applied universally to all JUnit3 tests. For tests that are affected, those tests are moved to JUnit5 base test. Part of eclipse-cdt#117

It may be that this test fails regularly because another test is not cleaned up properly. Make sure there are no unexpected projects in the workspace. Part of #117

Part of #117

This old test had a race condition. The failing test was trying to verify that CDTPROJECT_ADDED was received, but if there was a delay in the startup then another event would come in later. So for this test use the first received event, for the remaining tests use the last received event. Part of #117

On GitHub actions the org.eclipse.cdt.ui.tests.text.contentassist2 tests are running after ProposalFilterPreferencesTest and ProposalFilterPreferencesTest was changing the default filter and not restoring it. Part of #117

Part of #117

@AfterEach

Various tests are not cleaning up properly after themselves, causing test failures on subsequent tests. Therefore start each test by deleting all projects. In addition, some tests were creating their test projects in their constructor. As all the constructors run before all the tests as part of test discovery it means that projects were being created in constructor and interfering with other tests later. With the deleting of all projects in @AfterEach these tests would have started failing. Therefore, change these tests to create their projects and do other initialize tasks in the setUp method. For older JUnit3 style tests: This substantially slows down tests as many tests rely on sharing the project between multiple tests and recreating those projects on each run is slow. Therefore this is not applied universally to all JUnit3 tests. For tests that are affected, those tests are moved to JUnit5 base test. Part of #117

I saw in one test run this raw output for testCommonSDK: 50% 1/2 sources, 0 headers: parsing source.cpp (/baz1666753431025) which I think means that the indexing was not done for source.cpp so apply this pattern to testCommonSDK and IndexSearchTest both have intermittent failures on GitHub actions but pass fine locally. Part of eclipse-cdt#117

Part of eclipse-cdt#117

In 56ee2c3 I got github actions working by using default GDB on GHA, but on Jenkins we should continue to use CDT's pre-built version of GDB. Part of eclipse-cdt#117

This will allow, if needed, to mark tests as flaky Part of eclipse-cdt#117

Part of eclipse-cdt#117

Some tests sort of clean up after themselves, but still leave files around. Part of eclipse-cdt#117

Maybe once upon a time this lifecycle did something, but now in setUp fProject is always null and therefore the project was never getting deleted as the fProject that deleteProject saw was different than the tests. Part of eclipse-cdt#117

Part of eclipse-cdt#117

On GHA the error does not really give a good indication of what went wrong with minimal to no stack traces Part of #117

Ideally the code itself should also be deleted from CDT, but this test is super flaky and I cannot seem to convert it to JUnit5 so I can properly mark it as flaky. Therefore the test is now simply gone. Part of #117

Some of these tests left behind projects, by chaning them to extend BaseTestCase5 the resource cleanup happens and the tests are cleaned up properly Part of #117

This code seems to be trying to optimize across tests. This change isolated each individual test better. Also removed is a bunch of effectively unused test code. Part of #117

Part of #117

This is an ancient (2004) test that does not apply, was never referenced in the Suites and whose name doesn't match standard pattern. It tried to import ancient versions of projects as well, which would kick off the project converter UI that can't be disabled from the core plug-in. All in all, this test adds nothing of value. Part of #117

JDT thinks this is a test and will run it in the IDE and display an error. But it is only used to compose other tests, by making it abstract the IDE won't see it anymore. Part of #117

And by "new" I mean 15 years old. Part of #117

While it may be that the tests don't directly rely on JUnit5, the IDE requires JUnit5 in the classpath or else the launch config doesn't work with this error: Cannot find class 'org.junit.platform.commons.annotation.Testable' on project build path. Part of #117

Make sure not to lose stack trace when a test fails. Part of #117

Part of #117

Also stop catching exceptions and discarding stacktraces Part of #117

The test "abused" the JUnit3 ability to control test order and required all tests to run in the given order to work. Refactored the test to remove all redundant try/catch and provide a more traditional flow. Part of #117

Part of #117

Instead of using assertTrue, but assertEquals we get nice error messages that show the contents of the collections. Using assertEquals(List.of() instead of assertArrayEquals(... because that gives nicer error messages. Part of #117

Rather than update this test, simply delete this test of ancient functionality Part of #117

This provides the shared pre/post condition assertions to help make sure tests are running in a clean environment. If I had time I would convert all the tests to BaseTestCase5 which is JUnit5, but I only did the tests that had something wrong with them. Part of #117

This helps add some isolation between tests in case background threads are accessing a project. However I am not sure this solves any of the actual outstanding flaky tests. Part of #117

These tests all had the same mistake, they had 5 x waitForEvent with the comment "at this point all 5 threads should be stopped" but there are actually 6 threads (5 spawned ones + main thread). Most of the time all 6 threads would be stopped in time, but sometimes one of them wouldn't be stopped, leading to a "CoreException: Context is running" error in the test This fixes the ThreadStackFrameSyncTest item Part of #117

The format of this error message used to look like: ```java Expected number (0) of Non-OK status objects in log differs from actual (1). Error while parsing /projC_testTripleDownwardV/h3.h. java.lang.reflect.InvocationTargetException Caused by: java.lang.reflect.InvocationTargetException Caused by: java.lang.AssertionError: Need to hold a write lock to clear result caches ``` and it was hard to impossible to identify what the cause is. The hope is that capturing this fuller stack trace into the log will make it easier to identify the problem. Part of #117

jonahgraham · 2022-11-07T22:41:11Z

I have now fixed - or at least made a change which I think fixes - every flaky test that I have seen in the past few weeks.

Please raise new issues if you see other flaky tests. I would like to keep this number to 0 so that all PRs get green checks.

jonahgraham · 2023-09-15T14:16:09Z

testCommonSDK (org.eclipse.cdt.internal.pdom.tests.PDOMProviderTests) #120

Turns out this still fails on occasion - see #557

jonahgraham added a commit to jonahgraham/cdt that referenced this issue Oct 25, 2022

Add missing abstract keyword to abstract tests

bb14eaa

Part of eclipse-cdt#117

jonahgraham mentioned this issue Oct 25, 2022

Fix many of the test issues in #117 #118

Merged

jonahgraham added a commit to jonahgraham/cdt that referenced this issue Oct 25, 2022

Provide a way to wait on background build settings update job

19e64f3

Part of eclipse-cdt#117

jonahgraham added a commit to jonahgraham/cdt that referenced this issue Oct 26, 2022

Provide a way to wait on background build settings update job

b0313d1

Part of eclipse-cdt#117

jonahgraham added a commit that referenced this issue Oct 26, 2022

Add additional diagnostic to error message

a827d71

It may be that this test fails regularly because another test is not cleaned up properly. Make sure there are no unexpected projects in the workspace. Part of #117

jonahgraham added a commit that referenced this issue Oct 26, 2022

Add missing abstract keyword to abstract tests

128de5d

Part of #117

jonahgraham added a commit that referenced this issue Oct 26, 2022

Provide a way to wait on background build settings update job

ad478ce

Part of #117

jonahgraham mentioned this issue Oct 26, 2022

More test stability fixes #120

Merged

jonahgraham added a commit to jonahgraham/cdt that referenced this issue Oct 27, 2022

Typos and remove unused SuppressWarnings

c49896c

Part of eclipse-cdt#117

jonahgraham added a commit to jonahgraham/cdt that referenced this issue Oct 27, 2022

Convert pdom tests to JUnit5 format

bff6544

This will allow, if needed, to mark tests as flaky Part of eclipse-cdt#117

jonahgraham added a commit to jonahgraham/cdt that referenced this issue Oct 27, 2022

Reduce API warning in test

d7e9482

Part of eclipse-cdt#117

jonahgraham added a commit to jonahgraham/cdt that referenced this issue Oct 27, 2022

Fail test if there are any files left over in the workspace directory

b1ef0ed

Some tests sort of clean up after themselves, but still leave files around. Part of eclipse-cdt#117

jonahgraham added a commit to jonahgraham/cdt that referenced this issue Oct 27, 2022

Convert test to JUnit5 and cleanup projects made during test

b5d1b55

Part of eclipse-cdt#117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Better test error messages when workspace is not empty after test

a9c0153

On GHA the error does not really give a good indication of what went wrong with minimal to no stack traces Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Convert more of the tests to JUnit5

e1dea25

Some of these tests left behind projects, by chaning them to extend BaseTestCase5 the resource cleanup happens and the tests are cleaned up properly Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Delete projects fully at the end of tests

57d7a47

This code seems to be trying to optimize across tests. This change isolated each individual test better. Also removed is a bunch of effectively unused test code. Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Try to catch uncleaned up files earlier

a081987

Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

There is a new way to mark tests as failing

5cebec6

And by "new" I mean 15 years old. Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Pass exceptions up the stack

1cdf58a

Make sure not to lose stack trace when a test fails. Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Add missing abstract keyword to abstract tests

abd8147

Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Clean up created projects in tearDown

23399e8

Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Create temporary files with ResourceHelper so they get cleanedup

6ea5695

Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Delete settings.xml file at end of test

c650c25

Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Convert to JUnit5 and cleanup resources

05322ac

Also stop catching exceptions and discarding stacktraces Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Make sure test does not leave projects around

542e8b4

Part of #117

jonahgraham added a commit that referenced this issue Nov 7, 2022

Delete test for CDT 2.x functionality

021c036

Rather than update this test, simply delete this test of ancient functionality Part of #117

jonahgraham closed this as completed Nov 7, 2022

jonahgraham mentioned this issue Nov 11, 2022

Internal error while trying to serialize language settings Resource '/noFilesToBuild' does not exist. #163

Open

jonahgraham mentioned this issue Dec 5, 2022

org.eclipse.cdt.ui.tests.text.contentassist2.PlainC_CompletionTests #194

Open

jonahgraham added the releng Release engineering and project management label Jan 26, 2023

jonahgraham mentioned this issue Sep 15, 2023

testCommonSDK (org.eclipse.cdt.internal.pdom.tests.PDOMProviderTests) flaky #557

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test stability umbrella bug #117

Test stability umbrella bug #117

jonahgraham commented Oct 25, 2022 •

edited

Loading

jonahgraham commented Oct 25, 2022

jonahgraham commented Nov 7, 2022

jonahgraham commented Sep 15, 2023

Test stability umbrella bug #117

Test stability umbrella bug #117

Comments

jonahgraham commented Oct 25, 2022 • edited Loading

jonahgraham commented Oct 25, 2022

jonahgraham commented Nov 7, 2022

jonahgraham commented Sep 15, 2023

jonahgraham commented Oct 25, 2022 •

edited

Loading