Continue running the stress tester after an error is encountered #146
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Let’s see what kind of results we get out of this. Taking too long to run locally… @swift-ci Please test |
ahoppen
force-pushed
the
pr/dont-stop-on-error
branch
from
2fac180 to
7477138
Compare
ahoppen
changed the title
ahoppen
force-pushed
the
pr/dont-stop-on-error
branch
2 times, most recently
from
7ce71f3 to
b819dc4
Compare
`SourceKitDocument` should have reference semantics, because if we pass it around, we want all instances to refer to the same object.
…if suppress output is `false`
ahoppen
force-pushed
the
pr/dont-stop-on-error
branch
from
0d20196 to
220e520
Compare
ahoppen
changed the title
ahoppen
changed the title
ahoppen
force-pushed
the
pr/dont-stop-on-error
branch
2 times, most recently
from
d916129 to
2c9ac71
Compare
bnbarham
approved these changes
Jun 11, 2021
| let (actions, priorActions) = computeActions(from: tree) | ||
|
|
||
| var errors: [Error] = [] |
There was a problem hiding this comment.
This doesn't really matter, but I'd prefer the ones outside the loop to just return [error] and then only use this for the loop. Or use errors in the last do/catch as well.
Comment on lines
+137
to
+141
| // We can control whether to stop scheduling new operations here. As long | ||
| // as we return `true`, the stress tester continues to schedule new | ||
| // test operations. To stop at the first failure, return | ||
| // `operation.status.isPassed`. | ||
| return true |
There was a problem hiding this comment.
Could change FailFastOperationQueue to something like OperationQueue(failFast:). Just seems kind of weird to use a "failing fast" queue and then not fail fast 😆.
There was a problem hiding this comment.
I renamed it to StressTesterOperationQueue but left the API the same.
|
|
||
| private var state: State = .running { | ||
| didSet { | ||
| if #available(macOS 10.12, *) { |
There was a problem hiding this comment.
Is this still needed (since it is set in the package)?
Comment on lines
242
to
244
| if isNull { return false } | ||
| guard let notification = value.getOptional(.key_Notification)?.getUID() | ||
| else { return false } |
There was a problem hiding this comment.
Could factor this out into notificationType but there's only two, so not too bad right now.
The stress tester runs stable enough on the test suite now that a failure isn’t likely to cause a whole bunch of following failures. Instead, continue running after a failures is encountered to get a more complete picture of the SourceKit issues in the source compatibility suite. The current behaviour of uncovering new issues after XFailed issues are fixed is annoying.
ahoppen
force-pushed
the
pr/dont-stop-on-error
branch
from
2c9ac71 to
aa7bb02
Compare
ahoppen
changed the title
ahoppen
force-pushed
the
pr/dont-stop-on-error
branch
from
4365f15 to
de7fd46
Compare
We are no longer using this queue to fail fast, so the name seems misleading.
Shutdown an re-initialize would (sometimes?) give us back the previous XPC connection. The crash request is working even if SourceKit is busy fulfilling other requests and is guaranteed to spawn a new process.
ahoppen
force-pushed
the
pr/dont-stop-on-error
branch
from
de7fd46 to
3d6a913
Compare
|
@swift-ci Please test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The stress tester runs stable enough on the test suite now that a failure isn’t likely to cause a whole bunch of following failures.
Instead, continue running after a failures is encountered to get a more complete picture of the SourceKit issues in the source compatibility suite. The current behaviour of uncovering new issues after XFailed issues are fixed is annoying.