Conversation
pks-t
left a comment
pks-t
left a comment
There was a problem hiding this comment.
I'm generally 👍 on this, feels like it is long overdue. I've been wondering about whether this is the right layer to fix this, though. Wouldn't it make more sense to have this in the test code itself? Like that, developers would benefit from it, too, and we'd be able to mark single tests as flaky instead of having to re-do all tests of a particular test suite.
It's fine if your answer to the above is "no", though. :)
| $TestCommand += " -r${BuildDir}\results_${TestName}.xml" | ||
|
|
||
| Invoke-Expression $TestCommand | ||
| if ($LastExitCode -ne 0) { $global:Success = $false } |
There was a problem hiding this comment.
One thing I wondered about recently: do we really need the Powershell file at all? I saw that Azure does support Bash on Windows via Git for Windows. If that's the case, then we should drop this file completely so that we don't always have to keep both in sync. I'd naturally volunteer to do that.
There was a problem hiding this comment.
That's true, it does. I have no love for PowerShell... I'm happy to do that, too, but I've got a few other things going on before I can look at it.
Right. This was my first thought as well, actually. But then I realized it would require more thought than I wanted to give it. 😀 I think there are two issues here:
(But then I started yakshaving, because I wish clar's parser was smart enough to deal with commented out tests. So then I started thinking more about using llvm or something, and then things got really out of control.) We could also mark a suite as flaky on the command-line, which might actually be smarter still. eg,
🤷♂ So I decided to knock this out quickly so that I'd be less angry. 😉 |
|
|
||
| if [ "$FAILED" -ne 0 ]; then | ||
| SUCCESS=0 | ||
| fi |
There was a problem hiding this comment.
Previously we were executing failure, don't we have to do that now, too?
There was a problem hiding this comment.
Ach, that ended up dead. I removed it, but I also improved the error message a little, inspired by that function.
|
Fair enough ;) So let's get this merged quickly -- we can still improve in the future. |
ethomson
force-pushed
the
ethomson/flaky_ci
branch
3 times, most recently
from
b9f1625 to
1dc004e
Compare
Our online tests are occasionally flaky since they hit real network endpoints. Re-run them up to 5 times if they fail, to allow us to avoid having to fail the whole build.
Our online tests are occasionally flaky since they hit real network endpoints. Re-run them up to 5 times if they fail, to allow us to avoid having to fail the whole build.
ethomson
force-pushed
the
ethomson/flaky_ci
branch
from
1dc004e to
c7b4ce5
Compare
2 participants
Re-run the flaky online integration tests. Since these hit actual network endpoints and those may be occasionally down, we want to re-run them up to 5 times. This prevents us from failing the builds just because the badssl endpoint is down, for example.