[Core] Refactor Runtime

[mpkorstanje]

Summary

Extracting:

• Backend creation,
• Glue creation,
• Runner creation
• Feature compilation

from the runtime allows

• Tests on runners to skip the creation of Runtime
• Makes extracting Filter provider( Refactor Runtime.filters and matchesFilters out of Runtime #1352) and Feature provider(Refactor RuntimeOptions.cucumberFeatures out of RuntimeOptions #1366) easier.
• Will allow other runtimes such as JUnit and TestNg to skip the creation of the runtime
• Will facilitate parallel (Parallel support #1357), junit 5 (Add JUnit 5 Support #1149) and pickle runner support.
• Clarifies the execution loop of Cucumber

[boaty82]

Pulled the branch to work on some changes for PR 1357 and before I've made any changes I'm running the tests (whilst perusing the changes already made for me) and I get the following issue

[ERROR] Failed to execute goal org.revapi:revapi-maven-plugin:0.10.2:check (check) on project cucumber-java: The following API problems caused the build to fail:
[ERROR] java.annotation.noLongerPresent: @interface cucumber.api.java.ht.SipozeKe: Old API referenced the annotation type but the new API version no longer does. (breaks semantic versioning)
[ERROR] java.annotation.noLongerPresent: @interface cucumber.api.java.ht.SipozeKe: Old API referenced the annotation type but the new API version no longer does. (breaks semantic versioning)
[ERROR]
[ERROR] If you're using the semver-ignore extension, update your module's version to one compatible with the current changes (e.g. mvn package revapi:update-versions). If you want to explicitly ignore this change and provide a justification for it, add the following JSON snippet to your Revapi configuration under "revapi.ignore" path:
[ERROR] {
[ERROR] "code": "java.annotation.noLongerPresent",
[ERROR] "old": "@interface cucumber.api.java.ht.SipozeKe",
[ERROR] "package": "cucumber.api.java.ht",
[ERROR] "classQualifiedName": "cucumber.api.java.ht.SipozeKe",
[ERROR] "classSimpleName": "SipozeKe",
[ERROR] "elementKind": "@interface",
[ERROR] "justification": <<<<< ADD YOUR EXPLANATION FOR THE NECESSITY OF THIS CHANGE >>>>>
[ERROR] },
...

Just want to start with a clean running state, thanks in advance

[mpkorstanje]

Hey, feel free to comment out the revapi plugin. If you comment out the one in the plugins section, not the plugin management section it should be easy. Not sure what is causing this. Looks like the languages aren't generated.

…

On Tue, May 29, 2018, 12:17 boaty82 ***@***.***> wrote: Pulled the branch to work on some changes for PR 1357 <#1357> and before I've made any changes I'm running the tests (whilst perusing the changes already made for me) and I get the following issue [ERROR] Failed to execute goal org.revapi:revapi-maven-plugin:0.10.2:check (check) on project cucumber-java: The following API problems caused the build to fail: [ERROR] java.annotation.noLongerPresent: @interface cucumber.api.java.ht.SipozeKe: Old API referenced the annotation type but the new API version no longer does. (breaks semantic versioning) [ERROR] java.annotation.noLongerPresent: @interface cucumber.api.java.ht.SipozeKe: Old API referenced the annotation type but the new API version no longer does. (breaks semantic versioning) [ERROR] [ERROR] If you're using the semver-ignore extension, update your module's version to one compatible with the current changes (e.g. mvn package revapi:update-versions). If you want to explicitly ignore this change and provide a justification for it, add the following JSON snippet to your Revapi configuration under "revapi.ignore" path: [ERROR] { [ERROR] "code": "java.annotation.noLongerPresent", [ERROR] "old": ***@***.*** cucumber.api.java.ht.SipozeKe", [ERROR] "package": "cucumber.api.java.ht", [ERROR] "classQualifiedName": "cucumber.api.java.ht.SipozeKe", [ERROR] "classSimpleName": "SipozeKe", [ERROR] "elementKind": ***@***.***", [ERROR] "justification": <<<<< ADD YOUR EXPLANATION FOR THE NECESSITY OF THIS CHANGE >>>>> [ERROR] }, ... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1367 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGoAZxmQGuXZ3ie0Pj7TVHDZUUPdpDU9ks5t3SA8gaJpZM4UH0Fr> .

[mlvandijk]

@boaty82 You'll also have to comment out the enforcer (fyi ;))

[boaty82]

I’ve done that. Just thought there might be a more “proper” fix

[mpkorstanje]

No. Don't have a solution yet. There is something going wrong when you run the build. Maybe something executed in the wrong order that causes the languages not to be generated when revapi comes by.

[mpkorstanje]

I've moved the semantic version check to a different profile. Unless you turn it own it only runs on the CI.

This slows down the feedback loop a bit, but this is okay because we'll figure out which version it has to go in when merging. Not when building the feature.

[mpkorstanje]

Also feel free to ignore any problems with pax-exam.

[boaty82]

OK there are problems in the tests with queueing events, which I've been tracking down for a few hours across the last few days

Basically
Main
uses
RunnerSupplier runnerSupplier = new ThreadLocalRunnerSupplier(runtimeOptions, bus, backendSupplier, glueSupplier);

and ThreadLocalRunnerSupplier
return new Runner(glueSupplier.get(), eventBus.createBatchedEventBus(), backendSupplier.get(), runtimeOptions);
which creates the wrapping EventBus

However, TestHelper does

        RunnerSupplier runnerSupplier = new RunnerSupplier () {
            @Override
            public Runner get() {
                return new Runner(glue, bus, backendSupplier.get(), runtimeOptions);
            }
        };

which doesn't create the wrapper, which it should as this class should be effectively verifying what Main does

So if I update the TestHelper to do the same and then run cucumber.runtime.formatter.JSONFormatterTest#should_format_feature_with_background, for example, then the test fails. Having looked through the resulting json the step durations are all 0

I've tracked the problem down to StepDurationTimeService in that it registers for TestStepStarted events, and when it receives them it increments the currentTime by the required duration.

Just trying to think of a nice way to resolve this, but thoughts are welcome in the mean time.

Also, BTW the super.send in cucumber.runner.EventBus.BatchEventBus#send is kind of superfluous, in this instance, as the handlers in the BatchEventBus are empty and it isn't until parent.sendAll() when the events are distributed. But that isn't to say that the handlers would be empty in all scenarios

[boaty82]

Think I've found a nice solution.

I've created a new event TimerEvent, which StepDurationTimeService now registers for.
Updated TestStepStarted to implement it

I then had to update EventBus to loop through the handler map and check if the incoming event is an instance of the class in the key, rather than getting the key.

Just got to double check the tests

[mpkorstanje]

Hey Boaty, I'd rather not introduce a new event. I don't think it is needed. The current tests for the formatters are setup such that cucumber generates a stream of events. For tests we mock the time service so we can tick the clock manually and get predictable timestamps. The generation of these events is currently closely tied to the rest of the implementation and the tests. This was okay in the past. There was only one ordering of events. Now we have multiple orderings to deal with. Generating these in a controlled fashion requires first that we make the concept of an event stream explicit. An event stream is a sequence of events with some ordering: [TestStarted, TestSourceRead, {[ScenarioStarted, Step1, Step2, ScenarioFinished]*}, TestFinished] [This is in order], {these can come in any order}, these* can repeat. So by recording a regular event stream and shuffling the groups of scenario events we can simulate the result of a concurrent execution. This might also make the implementation of formatters simpler. Rather then trying to make all the formatters smart, we can record the events and replay them in the right order (sort the groups of scenario events by uri).

…

-MP PS: Can't look at the code. On a boat.

On Sat, Jun 9, 2018, 09:59 boaty82 ***@***.***> wrote: I've found a nice solution. I've created a new event TimerEvent, which StepDurationTimeService now registers for. Updated TestStepStarted to implement it I then had to update EventBus to loop through the handler map and check if the incoming event is an instance of the class in the key, rather than getting the key. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#1367 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGoAZ1v6qZKGBPHqpoo8Vfw_DVGo9LLMks5t64B7gaJpZM4UH0Fr> .

[boaty82]

Hi.

Haven't needed to add a new event interface in the end. Managed to just create a new EventBus implementation now, purely for tests, so TestStepStarted events are sent, all the time, for only certain EventListeners - in my case StepDurationTimeService - as well as being queued up for the Formatters

Have done the changes to support --threads, just adding the thread timeline report now. Do the HTML/JS/etc files need to go in a new jar & repo link the HTML plugin? I won't have permission to do this

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.