Remove the unnecessary execution.Scheduler.Init()
#2885
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2885 +/- ##
==========================================
- Coverage 76.75% 76.54% -0.21%
==========================================
Files 225 223 -2
Lines 16807 16796 -11
==========================================
- Hits 12900 12857 -43
- Misses 3080 3105 +25
- Partials 827 834 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
LGTM apart from the naming nitpick.
But to be honest I will likely need a day or two to remember all the intricacies, I knew, of this code to be certain that I don't forget about a corner case.
So I am hopeful that you working with it all the time means you would've caught it :P
There is no need to have 2 separate methods, Run() can start the VU initialization itself. As a bonus, this immediately makes the error handling around init errors much more in line with other error handling, allowing us to respect --linger and to try and execute handleSummary() if there were problems. Except the first init that is used to get the exported options, of course, that one is still special.
na--
force-pushed
the
remove-scheduler-init
branch
from
60c1026
to
10700cd
Compare
This whole sequence of 10+ PRs and tons of e2e tests has been mostly an attempt to simplify the old mess, so nobody has to remember all of its corner cases 😅 🤞 🙏 There is some unavoidable inherent complexity here, true, but most of the complexity before was accidental... 😞 That, coupled with all of the previously undefined behavior regarding exit codes and error handing (#2804), was a millstone around our neck - a huge piece of technical debt that we had to constantly work around... 😞 |
This reverts a big part of #2885 because with all of the logic in Run(), k6 didn't know if it needed to wait when --linger was specified. In general, if a script error or test.abort() occurs during init, --linger should not apply and k6 should exit immediately. But if test.abort() is called during the test run itself, or the test was stopped in some other way besides Ctrl+C, --linger means that k6 should not exit immediately after stopping the test.
This reverts a big part of #2885 because with all of the logic in Run(), k6 didn't know if it needed to wait when --linger was specified. In general, if a script error or test.abort() occurs during init, --linger should not apply and k6 should exit immediately. But if test.abort() is called during the test run itself, or the test was stopped in some other way besides Ctrl+C, --linger means that k6 should not exit immediately after stopping the test.
This is the first commit from #2815, plus an extra test that checks the exit code when aborting a
k6 run --pausedtest with Ctrl+C.Unfortunately, it turned out that 19dd505 from #2813 introduced a regression. Interrupting the k6 process with a signal should result in a
105exit code (ExternalAbort), but with that commit k6 exited with103if k6 was--paused. The good news is that the fix was already done in the first commit of #2815, which was entirely self-sufficient and can be moved to this separate PR. As a bonus, this makes reviewing everything even easier... 😅So, some details about the commit itself... 😅 In short, there is no need to have 2 separate
Run()andInit()methods in theexecution.Scheduler. Instead, Run() can start the VU initialization itself. As a bonus, this immediately makes the error handling around init errors much more in line with other error handling, allowing us to resolve the bug, but also to respect--lingerand to try and executehandleSummary()if there were problems. Except the first init that is used to get the exported options, of course, that one is still special.