Concept start end notify runner

[PiotrNestor]

Update to send concept start and end notification to a runner.
Add isConcept to step data.
Related to the corresponding gauge-proto update

@sriv @jensakejohansson
According to issue: getgauge/gauge-dotnet#203

[PiotrNestor]

@sriv
I cannot set go.mod to 'go 1.21'.
It reverts automatically to 'go 1.21.0' which is apparently not acceptable to CodeQL and golangci-lint
Is it because it's 1.21.0 in gauge-proto

[github-actions]

Benchmark Results

java_gradle_multithreaded.csv

Commit	CPU	Memory	Time	ExitCode
62b9937	10%	120752	0:23.40	0
b55b734	10%	108120	0:23.65	0
6475093	9%	114264	0:23.61	0
8479995	10%	127400	0:24.24	0

java_maven_serial.csv

Commit	CPU	Memory	Time	ExitCode
62b9937	80%	269976	0:17.24	0
b55b734	84%	216004	0:16.96	0
6475093	83%	260176	0:17.12	0
8479995	66%	271352	0:21.68	0

java_simple_multithreaded.csv

Commit	CPU	Memory	Time	ExitCode
62b9937	36%	70008	0:09.92	0
b55b734	38%	67900	0:09.27	0
6475093	30%	68364	0:12.14	0
8479995	36%	68064	0:09.63	0

java_gradle_serial.csv

Commit	CPU	Memory	Time	ExitCode
62b9937	9%	106932	0:24.90	0
b55b734	9%	118740	0:25.15	0
6475093	9%	117392	0:25.12	0
8479995	9%	99356	0:24.37	0

java_simple_parallel.csv

Commit	CPU	Memory	Time	ExitCode
62b9937	21%	72096	0:25.94	0
b55b734	24%	72212	0:22.78	0
6475093	22%	72064	0:23.98	0
8479995	21%	69924	0:26.32	0

java_gradle_parallel.csv

Commit	CPU	Memory	Time	ExitCode
62b9937	6%	119752	0:41.56	0
b55b734	5%	115740	0:40.07	0
6475093	5%	116668	0:46.13	0
8479995	6%	112568	0:40.00	0

java_maven_parallel.csv

Commit	CPU	Memory	Time	ExitCode
62b9937	41%	246160	0:33.10	0
b55b734	43%	231220	0:31.14	0
6475093	44%	243516	0:29.99	0
8479995	44%	235592	0:31.32	0

java_simple_serial.csv

Commit	CPU	Memory	Time	ExitCode
62b9937	42%	65868	0:14.17	0
b55b734	52%	66024	0:11.35	0
6475093	51%	65716	0:11.90	0
8479995	44%	69908	0:17.56	0

java_maven_multithreaded.csv

Commit	CPU	Memory	Time	ExitCode
62b9937	77%	234160	0:14.59	0
b55b734	67%	234752	0:16.79	0
6475093	58%	202856	0:19.71	0
8479995	65%	236972	0:17.15	0

Notes

• The results above are generated by running against seed projects in https://github.com/getgauge/gauge-benchmark
• These results are not persisted, but on merging to master the benchmark will be rerun.
• These benchmark are run in Github Actions' agents, which are virtualized. Results are not to be taken as actual benchmarks.Rather, these are indicative numbers and make sense for comparison.

---

See Workflow log for more details.

[sriv]

Is it because it's 1.21.0 in gauge-proto

Yes, I believe that could be a reason. We may need to set it to 1.21 in gauge-proto and check.

[PiotrNestor]

@sriv
After gauge-proto 'go 1.21' update
It was possible to correct to 'go 1.21'

OK, now it should work ...

[PiotrNestor]

@sriv
Go lint complains that a result should be verified

In notifyBeforeConcept and notifyAfterConcept
_, err := e.runner.ExecuteMessageWithTimeout(message)

Any suggestion what to do with that potential 'err' ?

[sriv]

Am yet to go into the details here, but what is the rationale for sending the concept start/end to runners?

[PiotrNestor]

@sriv
On the test side it's then possible to produce an informative test trace.
It provides better understanding of the test flow when a lot of concepts are used.

[sriv]

I see. But in order for the runner to pass the information to the test suite, you'd need new hooks for Before concept and After Concept with possibly some filtering?

I am not at my computer, but if you have any sample on how your test suite would consume this, perhaps I can offer some thoughts.

One thing to remember is that if gauge starts sending this message, all runners would get it and like we saw in the case of reporting plugins, we will break a lot of other runners since forward compatibility isn't yet there. The way to handle this is that runners have a feature flag capability in the json under 'capabilities'. We will need to check which runner is capable of receiving Concept hooks and send the message only for those which are ready.

Other than that, we may not need to use the ExecuteMessageWithTimeout directly, we can consider ExecuteAndGetStatus or an even more appropriate abstraction. I guess we can borrow the mechanism for steps and use it here

[PiotrNestor]

@sriv
This is discussed here:
getgauge/gauge-dotnet#203

The present 'gauge-dotnet' update is here:
https://github.com/system-verification/gauge-dotnet/tree/concept-start-end-notify-runner

Presently, the implementation in the runner is:

• accept the the Concept Start and End notifications
• call the existing Step related hooks
• add 'isConcept' == true if it's a concept step, == false otherwise
• also 'ErrorMessage' and 'StackTrace' are added to the StepInfo

Creating Concept related hooks would result in additional changes without providing better functionality or solution,

[jensakejohansson]

To elaborate a bit on @PiotrNestor answer above.

We have quite comprehensive test framework that produces a lots of informative logs about status of the test flows. We also have small "generic" steps that we potentially reuse many, many times within the same test scenario with differnt input data. In the log/trace we log the name of the current executing step (using the BeforeStep hook). Then the following log lines are then related to that step until next step name occurs, fine.

However, since the same step can be executed many times in a scenario and possibility be nested within a concepts it's not as trivial as it seems to easy correlate a log line to a specific point in the spec/flow. If we can log also what concept (potentially nested of course) the step was called from we can easily look at the log and see exactly "where we are" in the scenario.

The screenshot above that @PiotrNestor provided illustrates how we can now log that a step is part of concept, that is part of an other concept.

We though about a new hook, but decided against the complexity and now just added field that indicates if a step is concept or "real step". Then we catch this information in the BeforeStep hook. It may affect users of course, if they want to perform an action on steps but not concepts they have to filter based on the value of the IsConcept-field.

[sriv]

Thanks for the explanation @PiotrNestor and @jensakejohansson

I clearly need to focus more here. Apologies for going back and forth, but after the last release I am trying to be a bit more cautious here.

Here are my observations:

• In retrospect I am contemplating if these could be new hooks for below reasons:
  • Overloading BeforeStep and AfterStep may cause some confusion because the steps are not executed yet. In case of a concept the runner is not doing anything. So a BeforeConcept, AfterConcept may perhaps be semantically better?
  • The Before and After step hooks support the use of Message and CustomScreenshot which will not work here because the Concept ExecutionProcessors are returning an Empty response.
  • I am also worried that the existing BeforeStep and AfterStep will be invoked whenever there is a concept execution, which may not be ideal. i.e. with this change, gauge will send additional notiications for before/after concept, the runner would take it and set IsConceptStep=true and pass it to the test code via the Before/After step hooks. Which means the Before/After step hooks will be invoked once per step and once per every concept it is part of. This may or may not have a side effect depending on what the hook logic is. But I am worried that this changes the expected behaviour.
• The other thing I am thinking about is - right now I see that you are making changes to gauge, proto and dotnet runner. This will also impact the other language runners and we will see them erroring out for not understanding the ConceptStarting / ConceptEnding messages. My suggestion is to have a feature flag in the runners (see https://github.com/getgauge/gauge-dotnet/blob/master/src/dotnet.json#L38-L39 for examples) that lets gauge decide if it should send the new messages to the runner based on the capability of the runner.

Finally, regarding the go lint error - it depends on whether Concept hooks results need to be processed or not. I see an empty result presently returned your fork of gauge-dotnet, in which case something like _ := r.ExecuteAndGetStatus(message) would do well. See https://github.com/getgauge/gauge/blob/master/execution/specExecutor.go#L402 for a reference for how the other hooks are invoked.

[PiotrNestor]

@sriv
We'll proceed with the following updates then:

1. ExecuteAndGetStatus is used to send the Concept stop and end notifications
2. Add ConceptMessages flag to runner info in order to check if the runner supports these. This prevents sending the concept notifications to runners that don't know / want these.
3. Add hooks: BeforeConcept, AfterConcept. Use these for the Concept notifications in gauge-dotnet
4. Also, as a consequence isConcept is removed from gauge-proto StepInfo

[chadlwilson]

I haven't followed exactly what we are doing here, but given we broke compatibility with all the reporting plugins aside from html-report with 1.5.7 and haven't yet fixed this across all report plugins, we should definitely not break compatibility with any of the language runners or block them from gauge-proto upgrades - and should try and validate these.

Even with the feature flags, I would suggest we merge this without an immediate release so that we can then run builds across a few more of the various runners (which use gauge@master via gauge-tests) to gain confidence that they have not been broken? I believe the FTs run here only validate gauge-js and gauge-java.

[sriv]

The only feedback I have here (and I apologize for dragging this) is that these ConceptHooks are not sending any result back to the runner. As a result there will be no failure reported, no messages or screenshots will be accumulated.

This is perhaps obvious, but I want to make sure that you are aware of this.

This is not a show stopper as these can be added subsequently and need not make it in this PR. Please let me know if you are OK with this behaviour. Other than this this PR LGTM

[gaugebot]

@PiotrNestor Thank you for contributing to gauge. Your pull request has been labeled as a release candidate 🎉🎉.

Merging this PR will trigger a release.

Please bump up the version as part of this PR.

Instructions to bump the version can found at CONTRIBUTING.md

If the CONTRIBUTING.md file does not exist or does not include instructions about bumping up the version, please looks previous commits in git history to see what changes need to be done.

[chadlwilson]

(Off-topic FYI) The Python and Chocolatey releases worked fine after the release triggered by this, which is good.
https://community.chocolatey.org/packages/gauge
https://pypi.org/project/getgauge-cli/