Attach the build transcript and path to the build description to the test at the end of BuildOperationTester.checkBuild(). #978
Conversation
…test at the end of BuildOperationTester.checkBuild(). We can do this now that Swift Testing support attachments. Also give callers of checkBuild() the opportunity to opt out of adding these attachments, and have BuildTaskBehaviorTests.stressConcurrentCancellation() do so, as its particular nature can sometimes crash the tests when adding attachments.
mhrawdon
requested review from
aciidgh,
jakepetroules,
neonichu and
owenv
as code owners
|
@swift-ci test |
| let startBuilds = WaitCondition() | ||
| let buildsDone = WaitCondition() | ||
|
|
||
| // The threads this test spawns to run the build can outlive the lifetime of the test itself, so it passed attachBuildArifacts = false since if that happens then the test might crash because the reporter needed to add the attachments will no longer exist. |
There was a problem hiding this comment.
Whoa, that seems bad, nothing should outlive the test itself. I'm actually quite surprised this is here.
Can you make an array of Task<Void, Never> just before the for loop, and then append the Task handles to that array as you go? Then after the for loop, iterate the array and await the value of each of the tasks. That should guarantee we never have dangling work past the end of the test.
There was a problem hiding this comment.
I've already spent a couple of hours trying to fix up this test, and I'm not going to mess with it further as part of this PR. I suggest opening a new Issue for someone who is deeply familiar with it to investigate. I was able to reproduce it most of the time by running the whole BuildSystemTests suite inside of Xcode (removing the change to cause it to not add attachments, of course).
There was a problem hiding this comment.
Fair enough. Good that it's easy to reproduce.
| let startBuilds = WaitCondition() | ||
| let buildsDone = WaitCondition() | ||
|
|
||
| // The threads this test spawns to run the build can outlive the lifetime of the test itself, so it passed attachBuildArifacts = false since if that happens then the test might crash because the reporter needed to add the attachments will no longer exist. |
There was a problem hiding this comment.
Fair enough. Good that it's easy to reproduce.
2 participants
We can do this now that Swift Testing support attachments.
Also give callers of
checkBuild()the opportunity to opt out of adding these attachments, and haveBuildTaskBehaviorTests.stressConcurrentCancellation()do so, as its particular nature can sometimes crash the tests when adding attachments.