Simplify execution requirements calculation in k6 inspect
#2409
Conversation
na--
changed the title
k6 inspect
| @@ -347,9 +347,10 @@ func (r *Runner) GetOptions() lib.Options { | |||
|
|
|||
| // IsExecutable returns whether the given name is an exported and | |||
| // executable function in the script. | |||
| // | |||
| // TODO: completely remove this? | |||
There was a problem hiding this comment.
| // TODO: completely remove this? | |
| // TODO: Bundle.IsExecutable is now available, | |
| // it should be used directly and this method removed |
I think we could be a bit lost if we read the comment from outside of the context of this PR.
There was a problem hiding this comment.
Actually, I am not sure if we can remove this without a lot more refactoring 🤔 Bundle.IsExecutable is not available everywhere as a substitute... 🤔
There was a problem hiding this comment.
oh true, I always forgot about the Runner interface. Should we remove the comment? 🤔 It seems not really possible with the current code.
There was a problem hiding this comment.
I dug a little more here and realized that the need for the public method in Bundle disappears because of the other simplifications I did in #2412 with loadedTest 🎉 So I ended up reverting this part of the change there, see c3fec32
I think it's fine to merge this as it is and remove it in the other PR, given that it allows us to merge these PRs one by one instead of all at once.
I tried to read #1048 and honestly got lost in more than a few places... However, I am pretty sure this simplification is safe from reading through the code and thinking logically about it. There is no reason to create a runner and an execution scheduler to get the execution requirements a script. It should be enough to derive the
scenariosand just use theirGetFullExecutionRequirements()method, that's exactly what the execution scheduler does, after all... 😅k6/core/local/local.go
Line 69 in 1e28a3e
Anyway, tagged this as v0.38.0 since it doesn't really matter, but we should probably not introduce changes like this right before the release, even if they should not really change anything 😅 🤞