Fixes to try and improve stability of validation tests #112
Conversation
|
Close and reopen to trigger Appveyor. |
|
The build failed on Appveyor: https://ci.appveyor.com/project/wixtoolset/wix4/builds/42501364. The test |
|
Those tests fail for me locally as well. |
robmen
force-pushed
the
robmen/validation-test-fixes
branch
from
bfd5bd8
to
8320584
Compare
robmen
force-pushed
the
robmen/validation-test-fixes
branch
2 times, most recently
from
f5ddd3f
to
f015246
Compare
|
Did you see that yesterday's Appveyor build had a failing test ( |
Exceptions could cross process boundaries given the callback nature of validation so try to write error messages as soon as errors happen instead of throwing.
This should prevent validation from running in parallel.
When the validation tests are run repeatedly, they will expose an issue in the Windows Installer where ICE validations will occasionally fail to send all error/warning messages. The inconsistency obviously wreaks havoc on the tests. The tests are still interesting (and complex) enough to keep around for manual testing. So they were placed behind an #if for easy manual inclusion. Note that these are "negative tests" that expose the Windows Installer inconsistency when expected failures are missing. Other validation tests are "positive tests" that look for success and thus will always succeed. Therefore, the positive tests stay active.
robmen
force-pushed
the
robmen/validation-test-fixes
branch
from
d71b125
to
bca5c66
Compare
|
Committing based on build https://ci.appveyor.com/project/wixtoolset/wix4/builds/42533941 |
| @@ -193,7 +193,7 @@ string ReplacePathsInMessage(string message) | |||
| } | |||
| } | |||
|
|
|||
| [Theory(Skip = "Flaky")] | |||
| [Theory] | |||
There was a problem hiding this comment.
I don't know why you reenabled this test in this PR. This test flakiness is due to wixtoolset/issues#6720 and the associated MSBuild issue, not validation.
There was a problem hiding this comment.
You said the AppVeyor reproduced the issue. I re-enabled it to see if it reproduced. It didn't. If you still hit it intermittently due to not using the VS command prompt, do feel free to add the Skip back but be sure to include the link to the MSBuild issue since that has a good chance of living longer. I'm investigating wixtoolset/issues#6720 today.
There was a problem hiding this comment.
It worked because I changed Appveyor to temporarily use the VS command prompt: https://github.com/wixtoolset/wix4/pull/113/files#diff-92ab9a36df5d8e9f7076f2fdec59492d1ac2d9cf27ea046767a7fc4d542ef3dc. I expect it to start failing for everyone when 6720 is fixed.
There was a problem hiding this comment.
Awesome! Good to know I have a test already available to tell me when I have 6720 fixed. 👍🏻
There was a problem hiding this comment.
Well, yes. This test should have been failing this whole time. I'm pretty sure it was working properly when I set it up in the Tools micro repo, but either I didn't verify that it was actually running the 32-bit and 64-bit tasks or the MSBuild issue was introduced at the same time that we switched to the new repo.
No description provided.