Skip to content

[Frontend/Test] Fix flaky lock_interface test #36285

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

Merged
merged 2 commits into from
Mar 4, 2021

Conversation

xymus
Copy link
Contributor

@xymus xymus commented Mar 4, 2021

Testing that the compiler rebuilds the same module more than once with -disable-interface-lock was not reliable as some jobs could be executed after the module was rebuilt. Just make sure the compiler accepts the flag.

Also reorder permission change and FileCheck calls to make failures less likely to leave the tmp folder in an unwritable state.

This test was reenabled in #36185.

xymus added 2 commits March 4, 2021 09:42
Testing that the compiler rebuilds the same module more than once with
-disable-interface-lock was not reliable as some jobs could be executed
after the module was rebuilt. Just make sure the compiler accepts the
flag :/
@xymus
Copy link
Contributor Author

xymus commented Mar 4, 2021

@swift-ci Please smoke test

@xymus
Copy link
Contributor Author

xymus commented Mar 4, 2021

@swift-ci Please test Ubuntu 20.04 platform

@xymus
Copy link
Contributor Author

xymus commented Mar 4, 2021

@swift-ci Please test CentOS 8 platform

@xymus xymus changed the title [Frontent/Test] Fix flaky lock_interface test [Frontend/Test] Fix flaky lock_interface test Mar 4, 2021
@swift-ci
Copy link
Contributor

swift-ci commented Mar 4, 2021

Build failed
Swift Test Ubuntu 20.04 Platform
Git Sha - daf9af4

@xymus
Copy link
Contributor Author

xymus commented Mar 4, 2021

The Ubuntu failure is unrelated, the test succeeded on both GNU/Linux platforms.

@xymus xymus merged commit bea28e4 into swiftlang:main Mar 4, 2021
@xymus xymus deleted the fix-flaky-lock-test branch March 4, 2021 20:47
@davezarzycki
Copy link
Contributor

-j20 seems really arbitrary. What is the test trying to do?

@xymus
Copy link
Contributor Author

xymus commented Mar 4, 2021

It's trying to build many dependencies of Foo in parallel to make the implicit module build system try to rebuild Foo's module interface more than once. When the lock files are used it triggers a single rebuild, but without the lock files each of the frontend job can trigger its own rebuild. I believe the -j20 is to make sure the driver runs enough job in parallel for it to be noticeable.

@davezarzycki
Copy link
Contributor

  1. This sounds like a stress test, since success/failure depends on good/bad luck. It should therefore have REQUIRES: stress_test in the test.
  2. Why do you think -j20 is the right number? Why not -j2 or -j2000?

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.

4 participants