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

Fix some flaky managedbuilder tests #54

Merged
merged 2 commits into from Aug 22, 2022

Conversation

cwalther
Copy link
Contributor

This fixes some tests that sometimes fail for me when I run them in the Eclipse GUI on Linux.

  • CProjectDescriptionSerializationTests.testPersistentProperties contained a hardcoded sleep time to wait for an asynchronous operation that was often insufficient. Wait for the actual condition instead.

  • BuiltinSpecsDetectorTest.testAbstractBuiltinSpecsDetector_EnvChangesGlobal did not properly clean up after itself, which caused it, and some of its siblings, to fail when run for the second time in the same session.

(Running tests twice in the same session is not done deliberately, but it happens in Eclipse with a JUnit Plug-in Test launch configuration set to run all tests in org.eclipse.cdt.managedbuilder.core.tests/tests – once directly and once through AllLanguageSettingsProvidersMBSTestSuite. I have not found any way of running every test exactly once in the GUI – the recommended way seems to be to select org.eclipse.cdt.managedbuilder.tests.suite.AutomatedIntegrationSuite, but that leaves out some tests.)

Don't sleep for a hardcoded time but wait for the actual condition.
1000 ms was often insufficient on my system.
testAbstractBuiltinSpecsDetector_EnvChangesGlobal did not properly clean
up after itself, which caused
testAbstractBuiltinSpecsDetector_EnvChangesConfiguration_1,
testAbstractBuiltinSpecsDetector_EnvChangesConfiguration_2, and
testAbstractBuiltinSpecsDetector_EnvChangesGlobal to fail when run for
the second time in the same session.

Running tests twice is not useful, but happens in Eclipse with a launch
configuration set to run all tests in
org.eclipse.cdt.managedbuilder.core.tests/tests - once directly and once
through AllLanguageSettingsProvidersMBSTestSuite. I have not found any
way of running every test exactly once in the GUI.
@jonahgraham jonahgraham merged commit ef195db into eclipse-cdt:main Aug 22, 2022
@jonahgraham
Copy link
Member

Thanks @cwalther for fixing some of these tests.

The recommended way seems to be to select org.eclipse.cdt.managedbuilder.tests.suite.AutomatedIntegrationSuite, but that leaves out some tests.)

I think this is out of date now - the suites are still in there but we now run (most) tests using the standard patterns in Tycho Surefire. Some of the bundles have exceptions like this one:

<excludes>
<!-- The default Excludes omits nested static classes, this reenables them.
See org.eclipse.cdt.core.parser.tests.ast2.cxx14.constexpr.ArrayTests.NonIndexingTests
for an example of a test that would not be run otherwise.
For reference, the default exclude is "**/*$*"
-->
<exclude></exclude>
</excludes>

I'll try an update to the readme.

jonahgraham added a commit to jonahgraham/cdt that referenced this pull request Aug 22, 2022
as a result of the conversation on PR eclipse-cdt#54 we identified the FAQ
on this item was out of date.

eclipse-cdt#54 (comment)
jonahgraham added a commit to jonahgraham/cdt that referenced this pull request Aug 22, 2022
as a result of the conversation on PR eclipse-cdt#54 we identified the FAQ
on this item was out of date.

eclipse-cdt#54 (comment)
jonahgraham added a commit that referenced this pull request Aug 22, 2022
as a result of the conversation on PR #54 we identified the FAQ
on this item was out of date.

#54 (comment)
@cwalther
Copy link
Contributor Author

Thanks! This is working much more conveniently than Bugzilla+Gerrit.

@cwalther cwalther deleted the flakytests branch August 24, 2022 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants