Reqnroll
Support for Cucumber Messages #202
Replies: 27 comments 118 replies
-
|
@clrudolphi started the work on this:
|
Beta Was this translation helpful? Give feedback.
-
|
@gasparnagy I've made some restructuring of the Message Mapping spreadsheet that reflects my learnings so far. The timing of when we will trigger some Messages will be different than I originally presumed. At the start of each test (scenario row) we'll have to gather together info about which Step Defs will be bound to that scenario execution. Therefore the timing of TestCase and TestCaseStarted messages will have to be deferred until we see the first Step Execution event (and thus we know that binding is complete). Code for Message serialization and publication to an ndjson file is drafted and working in branch feature-CucumberMessages. Review comments welcome anytime, but it is a WIP and lots of changes still to come. |
Beta Was this translation helpful? Give feedback.
-
|
I can check them on Wed. |
Beta Was this translation helpful? Give feedback.
-
|
@clrudolphi Could you please make a draft PR from the |
Beta Was this translation helpful? Give feedback.
-
|
Draft PR: #233 |
Beta Was this translation helpful? Give feedback.
-
|
It might seem like an obvious question, but I would like to know: when would Reqnroll output Gherkin messages? |
Beta Was this translation helpful? Give feedback.
-
|
Yes, I meant "at what point in the dev/test lifecycle" - is it something that would be the output of feature parsing during the editing stage, or something emitted during test execution? |
Beta Was this translation helpful? Give feedback.
-
These are intended to be emitted during/after test execution. These should provide (at least, if not more) the info required of a tool similar to LivingDoc. |
Beta Was this translation helpful? Give feedback.
-
|
@clrudolphi About the plugin/not plugin question: Making it a plugin would be the logical choice for the reasons you mentioned, but since it will integrate so much into the core (because the need of source information) and in long term be used by other things, like reporting, probably we should integrate it into the product, but only activate (ie subscribe to the events, etc) if the user requires it (e.g. by setting a config setting). This is especially true, because as far as I see it will not involve any new dependency, so if we don't use its code, it will not cause any harm. |
Beta Was this translation helpful? Give feedback.
-
|
What type of output persistence mechanisms need to be supported? I haven't yet had a chance to survey the Cucumber ecosystem to see how consumers of Cucumber Message streams expect to receive them. Is persisting each Feature's worth of execution Messages into a file sufficient? Or should we allow for other/pluggable output mechanisms (such as database storage or webhooks)? |
Beta Was this translation helpful? Give feedback.
-
|
As far as I remember, Cucumbers only allow file output for not, so as a first step that is sufficient. |
Beta Was this translation helpful? Give feedback.
-
|
I have another question I would like to drop in: How do we handle parallel execution?
|
Beta Was this translation helpful? Give feedback.
-
|
I'm a little concerned that this is building on concepts that tie it to a specific execution model, when we know that's something which is making parallel scenario execution much harder to achieve. It might be a good answer on today's codebase, but it might be that it's adding more hurdles to downstream problems. I would like to see at minimum an anti-corruption abstraction that divorces the message-publishing from it's execution model; it needs to make sense that this can work when scenarios are executing in parallel or serially. I'm thinking back to my earliest thoughts where this sounds like a very structured logging abstraction. I would expect it to work such that there are parallel publishers and singleton sinks, with some amount of buffering between them as a synchronization medium, and an opportunity for filtering/enrichment. When the test run is complete, that's when the sinks are allowed to drain and finish writing anything in that buffer and you get a cohesive output. |
Beta Was this translation helpful? Give feedback.
-
|
@Code-Grump I appreciate your concern. What I have today is close to your expectation of
I have a Publisher singleton which subscribes to a set of Feature, Scenario, and Step level ExecutionEvents. It routes these to per-Feature instance objects (and related objects) that track and accrete state as the scenario/test-case proceeds through its execution. When a test-case is complete, the final set of CucumberMessages is generated and given back to the Publisher. (We have to wait until test-case completion b/c in a few situations not all contextual information will be available until then) I am trying to be cognizant of threading issues while keeping this as straightforward as possible. The vast majority of the code is not dependent on the TestThreadContext nor on the TestThreadEventPublisher directly. Most references are to FeatureContext, ScenarioContext and ScenarioStepContext. The Publisher does grab an instance of a TestThreadEventPublisher in order to add EventHandlers to it. My concern/question is how to do that correctly. The Publisher is currently written as a plug-in, so it only has certain points in the start-up sequence in which hook into things. I'm looking for confirmation on how best to wire in EventHandlers for each test thread (as represented by its TestThreadEventPublisher). Or, confirmation that it is not possible as a plug-in, in which case I'll revise the implementation of the Publisher to that of a service that is resolved and invoked elsewhere during the test thread spin up process. (Most likely as you suggest as a parallel set of Publishers, one per test thread context). |
Beta Was this translation helpful? Give feedback.
-
|
I think I have mentioned it somewhere, that I generally disagree to the design decision that the |
Beta Was this translation helpful? Give feedback.
-
|
Is this the proper way for a singleton plug-in component to gain access to the ExecutionEvents for all "workers"? (Given current threading/parallelization models) public CucumberMessagePublisher(ICucumberMessageBroker CucumberMessageBroker, IObjectContainer objectContainer)
{
broker = CucumberMessageBroker;
}
public void Initialize(RuntimePluginEvents runtimePluginEvents, RuntimePluginParameters runtimePluginParameters, UnitTestProviderConfiguration unitTestProviderConfiguration)
{
runtimePluginEvents.CustomizeFeatureDependencies += (sender, args) =>
{
objectContainer = args.ObjectContainer;
var testThreadExecutionEventPublisher = args.ObjectContainer.Resolve<ITestThreadExecutionEventPublisher>();
HookIntoTestThreadExecutionEventPublisher(testThreadExecutionEventPublisher);
};
}
public void HookIntoTestThreadExecutionEventPublisher(ITestThreadExecutionEventPublisher testThreadEventPublisher)
{
testThreadEventPublisher.AddHandler<FeatureStartedEvent>(FeatureStartedEventHandler);
testThreadEventPublisher.AddHandler<FeatureFinishedEvent>(FeatureFinishedEventHandler);
testThreadEventPublisher.AddHandler<ScenarioStartedEvent>(ScenarioStartedEventHandler);
testThreadEventPublisher.AddHandler<ScenarioFinishedEvent>(ScenarioFinishedEventHandler);
testThreadEventPublisher.AddHandler<StepStartedEvent>(StepStartedEventHandler);
testThreadEventPublisher.AddHandler<StepFinishedEvent>(StepFinishedEventHandler);
testThreadEventPublisher.AddHandler<HookBindingStartedEvent>(HookBindingStartedEventHandler);
testThreadEventPublisher.AddHandler<HookBindingFinishedEvent>(HookBindingFinishedEventHandler);
testThreadEventPublisher.AddHandler<AttachmentAddedEvent>(AttachmentAddedEventHandler);
testThreadEventPublisher.AddHandler<OutputAddedEvent>(OutputAddedEventHandler);
}This class is the root of my implementation. It is a "built-in" plugin (in that it implements the IRuntimePlugin interface, but is built-in to the Reqnroll project and is not packaged separately). I have it registered in the global container as a singleton. I'm taking advantage of the feature of the plug-in loader in that it initializes anything in the global container that implements the plugin interface. |
Beta Was this translation helpful? Give feedback.
-
|
This approach is not ideal, but I don't have a perfect choice for now. The problem with this approach is that if you resolve any interface (in this case A better approach (also not perfect) would be to use the The best would be to add a Before/After TestThread events to the |
Beta Was this translation helpful? Give feedback.
-
I don't believe the CCK has any examples that specifically cover this. |
Beta Was this translation helpful? Give feedback.
-
|
Requirements clarification When we use the Gherkin Pickle Compiler on this scenario text, it fails b/c there is no Table to provide values for the "product" variable.
|
Beta Was this translation helpful? Give feedback.
-
|
In my branch for the Messages I have taken the approach of modifying the External Data plugin to provide the Location (0,0) elements that are missing. |
Beta Was this translation helpful? Give feedback.
-
|
This is a hard question. Ideally I would convince the Messages protocol to make location optional (and fix the Fixing the External Data plugin is only good as a temporary solution, as there might be others as well using null locations as there was no such constraint in our AST so far. Making a routine that patches up AST is also a good temporary solution (but maybe too much work). |
Beta Was this translation helpful? Give feedback.
-
|
I was able to borrow the same technique that the External Data Plugin uses for Scenario Transformation (ie, GhekinDocumentVisitor and ScenarioTransformation classes) within the Parser project. This is then invoked during Generation just before I invoke the AstMessagesConverter. Should the External Data Plugin then be refactored to leverage the visitor and scenario transformer from the Parser rather than duplicating this code across the two projects? |
Beta Was this translation helpful? Give feedback.
-
|
Generally: yes, but if you feel that it is too much for you right now, just leave a comment at the external data plugin and we will do it later in a separate PR. |
Beta Was this translation helpful? Give feedback.
-
|
Resolved: I've applied the same adjustments to the External Data plugin. |
Beta Was this translation helpful? Give feedback.
-
|
Design Question about Bindings. When a step is ambiguous, Cucumber Messages wants to include the detail about all of the StepDefinitions that match the step text. Are those available as StepDefinitionBinding objects during a StepStarted or StepFinishedEvent? Or during one of the step Binding events? |
Beta Was this translation helpful? Give feedback.
-
|
The But I think we have bigger problems, as I think the StepStarted/StepFinishedEvent is not even invoked for erroneous matches (ambiguity, parameter error). So I fear there is quite a big change required for supporting this. Can we skip handling this for now? |
Beta Was this translation helpful? Give feedback.
-
|
I've researched this further. The StepStarted/StepFinished events do get fired. The StepFinishedEvent (after earlier modifications for Messages) gives me the ScenarioContext, and from that, I can get the BindingException that was thrown. |
Beta Was this translation helpful? Give feedback.
-
Yes, this is a good idea. |
Beta Was this translation helpful? Give feedback.
-
|
Resolved: created sub class called AmbiguousBindingException. |
Beta Was this translation helpful? Give feedback.
-
|
Implementation Question: the Cucumber Message called Meta holds information about the name and version of the SUT, the runtime (in our case .NET, v8), the OS, and the CPU; along with info about the git commit and the CI build. |
Beta Was this translation helpful? Give feedback.
-
|
Not critical. Currently we have this logic in the |
Beta Was this translation helpful? Give feedback.
-
|
Resolved: refactored the relevant code out of AnalyticsProvider. |
Beta Was this translation helpful? Give feedback.
-
|
CCK alignment question - Skipped Scenarios. The CCK includes a set of scenarios to demonstrate how 'skip' works and the Cucumber Messages that result. The first such example from the CCK is this: The '@Skip' tag invokes a BeforeScenario hook that signals that the scenario should be skipped (in the provided Typescript example implementation). Reqnroll doesn't support programmatically skipping scenarios from within Hooks (according to this part of the documentation ). I have experimented by changing the 'skip' tag into an 'ignore' tag. That results in a set of Cucumber Messages that seriously diverges from the CCK. The ignore tag causes Reqnroll to completely jump over the scenario, with no Messages as a result, while the CCK actually executes the scenario, at least as far as that 'skip' hook goes with a set of resulting Messages (TestCase, TestStart, TestStep, etc). I'm open to suggestions on how to proceed. |
Beta Was this translation helpful? Give feedback.
-
|
Can we skip supporting this test case from CCK? I don't think we have a matching feature for this. |
Beta Was this translation helpful? Give feedback.
-
Lol. I see what you did there. |
Beta Was this translation helpful? Give feedback.
-
|
Out of Scope |
Beta Was this translation helpful? Give feedback.
-
|
Implementation Questions: Attachments - Cucumber Messages schema requires that when an attachment is added that the contents of the attachment be included with the Messages (typically as BASE64). It also requires a MIME type. First question: when the ExecutionEvent AttachmentAddedEvent fires there is no context about which Feature or Scenario to which this event is associated. How do you suggest we make that connection so that the info about the attachment can be inserted into the correct Message stream? Second question: The Reqnroll event for an added attachment includes only the file name, which means that the MIME type must be inferred or detected by inspection. |
Beta Was this translation helpful? Give feedback.
-
|
I guess on "direct dependency" you mean that the relevant context should be passed in to the "entry" method, in this case to the |
Beta Was this translation helpful? Give feedback.
-
|
I tried various approaches. I first tried adding IFeatureContext, IScenarioContext, and IScenarioStepContext in the constructor of ReqnrollOutputHelper. That didn't work as these aren't available when the ReqnrollOutputHelper is first resolved. |
Beta Was this translation helpful? Give feedback.
-
|
On a side note as I was digging around. The MS DI plug-in seems to resolve an instance of the ReqnrollOutputHelper during its startup sequence. I'm not familiar with that DI framework but wouldn't that mess up the lifetime management of the ReqnrollOutputHelper and accessibility to dependencies? |
Beta Was this translation helpful? Give feedback.
-
I'm also not familiar to this, but this might be problematic. Could you please create a separate issue for checking this before we forgot? |
Beta Was this translation helpful? Give feedback.
-
|
After further review I no longer think this is a problem. I was misreading the code. |
Beta Was this translation helpful? Give feedback.
-
|
Design/Implementation Question: alignment of hook execution to Features and Scenarios.
There are two Hook related ExecutionEvents I can subscribe to: HookStarted/Finished and HookBindingStarted/Finished. The HookStarted event includes info on which Feature and Scenario is involved, but only includes the HookType of the hooks that will be invoked. The HookBinding event indicates which specific Hook class/method is being invoked, but doesn't include enough context to know which test execution thread from which it is coming. When multiple Features are running simultaneously (and/or multiple Scenarios in parallel within a Feature), we don't have enough context to piece together the relationship between a fired Hook and the feature/scenario/step to which it belongs. Would anyone have any concerns with a change to the HookBindingStarted/Finished events payload to include Feature/Scenario/Step context? |
Beta Was this translation helpful? Give feedback.
-
|
No, feel free to change this as it is practical for you. |
Beta Was this translation helpful? Give feedback.
-
|
Resolved: HookBindingStarted and Finished constructors changed to take relevant context info. |
Beta Was this translation helpful? Give feedback.
-
|
The other Cucumber implementations have, and the CCK expects, the ability to set a (optional) "Name" on a hook. Reqnroll doesn't support this and for now our Cucumber Messages implementation won't be strictly compliant in that regard. |
Beta Was this translation helpful? Give feedback.
-
|
Can't speak to the history, but it feels like it may be a feature that nobody ever asked for. I'm actually not sure I can think of place this would be useful in the test suites I've worked on, so it might be a superfluous part of the spec that nobody really uses or only shows up in some specific edge-cases with external tooling. In either case, can we interpret the spec and act as though it's not been specified (as it's an optional field and we do not provide a means to specify it?) |
Beta Was this translation helpful? Give feedback.
-
Yes, the name attribute is optional. So we can proceed with implementation of Messages. The Messages that our implementation will create will be compliant. But our implementation will not pass the compatibility test as laid out in the CCK for hooks. This won't be the only area within the CCK for which we'll be non-compliant as there are other feature sets that Reqnroll doesn't support. |
Beta Was this translation helpful? Give feedback.
-
|
History: This is the first time I heard about hook names (for any cucumber tool). I can imagine that some of them had this and was added to the protocol to preserve it. This is an interesting topic. When we (actually @clrudolphi, if I remember well 😀) did the "go to hook" feature in the Visual Studio, we noticed that it is not obvious how to refer to a hook when we present it to the users. Finally we have chosen to use the class+method name. I wonder if the hooks would have a human readable name, it would be easier for the users to understand what is happening. But I would separate this idea from the Cucumber Messages and implement it separately if we really want. @clrudolphi Can we maybe use the class+method name as a name for the hook? |
Beta Was this translation helpful? Give feedback.
-
|
Indeed for the Messages plug-in I am using the class name, method name and list of parameter type names as the hook name. |
Beta Was this translation helpful? Give feedback.
-
|
StepArgumentTransformations: the analogue within Messages of a StepArgumentTransformation is called a ParameterType. ParameterType's have two boolean properties: preferForRegularExpressionMatch and useForSnippets. I'm not seeing an obvious mapping from StepArgumentTransformation for these values. |
Beta Was this translation helpful? Give feedback.
-
|
IDK either. @gasparnagy? |
Beta Was this translation helpful? Give feedback.
-
|
The The Although the But in Cucumbers you would need to combine this into a single parameter type and merge the regexes. |
Beta Was this translation helpful? Give feedback.
-
|
If it's ever actually useful to have a closer mapping to these concepts, we could probably build some kind of |
Beta Was this translation helpful? Give feedback.
-
|
Yep, I agree. |
Beta Was this translation helpful? Give feedback.
-
|
Resolved: hard coded these values for now. |
Beta Was this translation helpful? Give feedback.
-
|
@gasparnagy Would you please look at the interplay between methods OnScenarioEndAsync() and FireEventsAsync() in the TestExecutionEngine? It seems to me that if a user-hook throws an exception in an AfterScenario hook, then the ScenarioFinishedEvent is never fired. public virtual async Task OnScenarioEndAsync()
{
try
{
if (_contextManager.ScenarioContext.ScenarioExecutionStatus != ScenarioExecutionStatus.Skipped)
{
await FireScenarioEventsAsync(HookType.AfterScenario);
}
_testThreadExecutionEventPublisher.PublishEvent(new ScenarioFinishedEvent(FeatureContext, ScenarioContext));
}
finally
{
_contextManager.CleanupScenarioContext();
}
}Would it be as simple as moving the invocation of the PublishEvent() to the finally block? |
Beta Was this translation helpful? Give feedback.
-
|
Now we're in the realm of asking what these events really mean. Does If we want an event that means "there is no more code to execute in the scenario" it might be important to include a indication of whether the scenario execution completed successfully and to publish the exception (if there was one). This feels like a significant change from the current model. What's the expectation of Cucumber Messages when the runtime fails? Not that the scenario threw an error, but that something in the test engine itself had a problem? |
Beta Was this translation helpful? Give feedback.
-
|
There is no expectation within Messages about runtime or test infrastructure failures. Within Reqnroll, care is (normally) taken to ensure that exceptions thrown within user-code do not interfere with the proper execution of Reqnroll. The exceptions are caught, then later attached to the Scenario outcome (and affect scenario execution status appropriately etc). This is the only situation I've encountered in which a user-code exception would interfere with the continued execution of Reqnroll. The disruption is minor in that there are currently very few elements that subscribe to the ScenarioFinishedEvent; therefore I'm not really surprised that we haven't heard any complaints to-date. My expectation (and I believe the intuitive one) would be that the ScenarioFinishedEvent would fire regardless of any user-level code exceptions. |
Beta Was this translation helpful? Give feedback.
-
|
Makes sense. I'm just repeating myself: the introduction of the current event stuff was done for supporting DivingDoc gen and I'm afraid there was no detailed design consideration behind the individual decisions. So in short: I would not be worried about backwards compatibility for these. |
Beta Was this translation helpful? Give feedback.
-
|
Resolved: refactored so that the event is fired during the finally clause. |
Beta Was this translation helpful? Give feedback.
-
|
Another side issue (perhaps?). The Meta message would like to include the name and version of the Gherkin tool being used. In our case, we would be providing a property of the Meta message called Implementation with sub attributes of "name":"Reqnroll" and "version":"2.1.0". private string GetReqnrollVersion()
{
return VersionInfo.AssemblyInformationalVersion;
}The VersionInfo class pulls info from the EntryAssembly and pulls from the AssemblyInformationVersionAttribute to obtain the version info. internal class VersionInfo
{
internal static string AssemblyVersion { get; }
internal static string AssemblyFileVersion { get; }
internal static string AssemblyInformationalVersion { get; }
internal static string NuGetVersion { get; }
static VersionInfo()
{
var assembly = Assembly.GetEntryAssembly() ?? Assembly.GetExecutingAssembly();
AssemblyVersion = assembly.GetName().Version.ToString();
AssemblyFileVersion = assembly.GetCustomAttribute<AssemblyFileVersionAttribute>()?.Version;
AssemblyInformationalVersion = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion;
NuGetVersion = AssemblyInformationalVersion?.Split(new[] { '+' }, 2)[0];
}
}When I use the above code paths to generate a Meta message from within the TestExplorer of VisualStudio I end up with a version attribute of: "version":"16.4.0", which is the version number of the microsoft.net.test.sdk, not that of Reqnroll. Same result when I execute via dotnet test on the command line directly. Is there some other mechanism that I should be using? |
Beta Was this translation helpful? Give feedback.
-
|
The "entry assembly" is the assembly that was the first one loaded by the runtime, usually the entry point for the application. Reqnroll is loaded by a test framework, so it will never be the entry assembly. If what you want is the version information for Reqnroll, we can easy find that assembly using |
Beta Was this translation helpful? Give feedback.
-
|
Exactly. Which leads me to believe that the VersionInfo class implementation is wrong I'm using GetExecutingAssembly. (Which has been in the main branch of Reqnroll for months) I'll submit a separate PR to correct it. |
Beta Was this translation helpful? Give feedback.
-
|
In the PR, could you indicate places where it's currently used? It'd be good to understand any knock-on it might cause. It seems unlikely that anything significant is using it right now, given how wrong it appears to be. |
Beta Was this translation helpful? Give feedback.
-
|
Submitted #248 |
Beta Was this translation helpful? Give feedback.
-
|
Scope Expressions: After('@some-tag or @some-other-tag', function () {
throw new Error('Exception in conditional hook')
})Reqnroll doesn't support this, right? We would expect the hook developer to use two Scope attributes or look at the Tags property of the ScenarioContext within the code of the hook. This will result in a discrepancy in the Messages emitted for that CCK test case.
I'm not suggesting we attempt to 'fix' this; just raising awareness of the discrepancy. |
Beta Was this translation helpful? Give feedback.
-
|
Thx, your analysis is correct. A small addition to it: in Reqnroll one can also use the hook attributes with multiple tag parameter ( At some point it would be great to support tag expressions like Cucumber, but not critical. |
Beta Was this translation helpful? Give feedback.
-
|
Another related issue: the Messages CCK renders tags with the @ prefix. That presumably means that the Cucumber ecosystem of tools expects tags to have the @-sign prefix in order to considered a valid tag. |
Beta Was this translation helpful? Give feedback.
-
|
I feel like the rendered output should change to match, but we don't use the |
Beta Was this translation helpful? Give feedback.
-
|
To clarify, I'm talking about the scopes applied to Hooks. The CCK expects that the scopes use the same convention as tags do and use an @-sign prefix. Sorry for the confusion. |
Beta Was this translation helpful? Give feedback.
-
|
Ok. Please add back the |
Beta Was this translation helpful? Give feedback.
-
|
Another minor difference, would like confirmation that I can ignore it: In GherkinDocuments, the language of the document is specified using typical ISO monikers, like 'en-US' etc. The CCK is giving only the language portion of this, not the culture suffix. So in the previous example, the expectation is that the GherkinDocument would have a language attribute of 'en'. The C# Gherkin parser is the component creating this (to my understanding). |
Beta Was this translation helpful? Give feedback.
-
|
Yes, for .NET we need a full culture (en-US), a culture neutral language (en) is not enough, therefore we have a mechanism in the Gherkin parser to "extend" the Gherkin language variations with some default culture (e.g. from "en" we make "en-US"). But there are some languages in Gherkin (eg. We can ignore it for now, the proper solution would be if the Gherkin parser would remember somehow the original Gherkin language marker as well as the one we calculate. But not that important probably. |
Beta Was this translation helpful? Give feedback.
-
|
The CCK demonstrates that when Cucumber parses a step and binds it to a stepDef, it captures and reports the individual group matches that make up the arguments that will be passed to the StepDef. In most situations Reqnroll will be able to provide this (one value per argument). |
Beta Was this translation helpful? Give feedback.
-
|
Agree. Out of scope. |
Beta Was this translation helpful? Give feedback.
-
|
Another Hook concern: order of execution of AfterScenario hooks. According to the Gherkin team over on Discord, the expected order of execution of After hooks is in reverse order of their definition (absent any other explicit ordering). For the Messages implementation, I propose ignoring this and skip validation against the CCK for the ordering of hooks. |
Beta Was this translation helpful? Give feedback.
-
|
OK. Reqnroll (and SpecFlow) behavior for ordering "after" hooks is different. There was a long discussion and debate on this when we introduced the order. Cucumber uses the "order" word, but uses it as a priority (hence reversed for "after" hooks). We decided on a better language clarity and "order" is always an order. Unfortunately this was quite long after the Cucumber implementation has been finished, so changing it in Cucumber was anyway out of question so we decided to keep this as an acknowledged small difference in the frameworks. So yes, please ignore that test. |
Beta Was this translation helpful? Give feedback.
-
|
I could use some pointers/help on how to deal with file-based IO that is needed during test execution during a Github build. The Messages will be persisted to a file. I have a rudimentary .config feature that provides a means for telling the Cucumber Messages plug-in where to store the output file(s). |
Beta Was this translation helpful? Give feedback.
-
|
@clrudolphi Do we need to generate a separate file per feature? I thought we would generate a file for the entire run. In that case, I would just generate a fixed name by default, like |
Beta Was this translation helpful? Give feedback.
-
|
There is no "requirement" per se. The CCK is given on a per feature basis. In the CCK the output file name is given as the feature file name plus .ndjson. So far, I have instead been using a sanitized version of the feature name. Would you have any experience on what the downstream ecosystem of tools expect to consume? |
Beta Was this translation helpful? Give feedback.
-
|
Most of the similar tools produce a single file with a fixed name. That is the easiest to process downstream, because with any variable names the scripts would need to guess what the output file was. So if there is no specs for that, I would go for a fixed name for now. |
Beta Was this translation helpful? Give feedback.
-
|
Re: configuration. Do we have an established mechanism by which the hosting Test Frameworks can pass configuration data on to Reqnroll? I'm thinking of a tiered system of configuration overrides: defaults, settings present in a config file, settings overridden by Environment Variables, and settings overridden when provided from the Test Host framework (such as from the command-line). The latter assumes that we have a way for the test framework to pass along the command line options to us. Does such an idea sound feasible and desirable? |
Beta Was this translation helpful? Give feedback.
-
|
@clrudolphi I really like the level of energy you put into this. Could you please move the configuration topic to a separate top level topic to the discussion board. I think that should be anyway implemented separately from the cucumber messages and it is probably too hidden here. |
Beta Was this translation helpful? Give feedback.
-
|
Design Question: @gasparnagy How open would you be to changes to the structure of the generated test classes such that the PickleIds are included as arguments to test methods? So, to take an example, here is what an Examples driven test would generate. Note the pickleIds as part of the DataRowAttribute and the use of a helper function to find a PickleStep.Id given a PickleId and step text: [Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute()]
[Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute("Valid product prices are calculated")]
[Microsoft.VisualStudio.TestTools.UnitTesting.TestPropertyAttribute("FeatureTitle", "External Data from CSV file")]
[Microsoft.VisualStudio.TestTools.UnitTesting.TestCategoryAttribute("DataSource:products.csv")]
[Microsoft.VisualStudio.TestTools.UnitTesting.DataRowAttribute("Chocolate", "2.5", null, "pickleId1")]
[Microsoft.VisualStudio.TestTools.UnitTesting.DataRowAttribute("Apple", "1.0", null, "pickleId2")]
[Microsoft.VisualStudio.TestTools.UnitTesting.DataRowAttribute("Orange", "1.2", null, "pickleId3")]
public async System.Threading.Tasks.Task ValidProductPricesAreCalculated(string product, string price, string[] exampleTags, string pickleId)
{
string[] @__tags = new string[] {
"DataSource:products.csv"};
if ((exampleTags != null))
{
@__tags = System.Linq.Enumerable.ToArray(System.Linq.Enumerable.Concat(@__tags, exampleTags));
}
string[] tagsOfScenario = @__tags;
System.Collections.Specialized.OrderedDictionary argumentsOfScenario = new System.Collections.Specialized.OrderedDictionary();
argumentsOfScenario.Add("product", product);
argumentsOfScenario.Add("price", price);
global::Reqnroll.ScenarioInfo scenarioInfo = new global::Reqnroll.ScenarioInfo("Valid product prices are calculated", "\tThe scenario will be treated as a scenario outline with the examples from the CS" +
"V file.", tagsOfScenario, argumentsOfScenario, featureTags, pickleId);
#line 4
this.ScenarioInitialize(scenarioInfo);
#line hidden
if ((global::Reqnroll.TagHelper.ContainsIgnoreTag(scenarioInfo.CombinedTags) || global::Reqnroll.TagHelper.ContainsIgnoreTag(featureTags)))
{
testRunner.SkipScenario();
}
else
{
await this.ScenarioStartAsync();
#line 6
string pickleStepId;
pickleStepId = FindPickleStepId(pickleId, string.Format("the customer has put 1 pcs of {0} to the basket", product));
await testRunner.GivenAsync(string.Format("the customer has put 1 pcs of {0} to the basket", product), ((string)(null)), ((global::Reqnroll.Table)(null)), pickleStepId, "Given ");
#line hidden
#line 7
pickleStepId = FindPickleStepId(pickleId, "the basket price is calculated");
await testRunner.WhenAsync("the basket price is calculated", ((string)(null)), ((global::Reqnroll.Table)(null)), pickleStepId, "When ");
#line hidden
#line 8
pickleStepId = FindPickleStepId(pickleId, "he basket price should be greater than zero");
await testRunner.ThenAsync("the basket price should be greater than zero", ((string)(null)), ((global::Reqnroll.Table)(null)), pickleStepId, "Then ");
#line hidden
}
await this.ScenarioCleanupAsync();
}With the above, the PickleId associated with each test case could be included with each relevant ExecutionEvent. This will allow a singleton subscriber to those events to be able to route those events to the correct component that is tracking the progress of that particular test case. |
Beta Was this translation helpful? Give feedback.
-
|
No other concerns. The performance will be an interesting topic, but I guess we need to see it first to have a feeling about it. If it will be an issue, we will need to see what can we do. |
Beta Was this translation helpful? Give feedback.
-
|
Generating IDs is a surprisingly complex problem. Do you know what scope they have to be unique in and when they should be the same? An incremental strategy is probably not ideal - a change to one scenario usually should not cause a change to another. |
Beta Was this translation helpful? Give feedback.
-
|
The requirements are simple. The generated IDs are unique within the scope of an execution of a Feature. There are no requirements that the IDs be stable or repeatable (neither across executions or across edits to features). |
Beta Was this translation helpful? Give feedback.
-
|
It might be reasonable to use a version-5 UUID with a suitable name. The more deterministic and independent, the better for deterministic source generation. |
Beta Was this translation helpful? Give feedback.
-
|
Resolved: generator assigns each test method an index, stored in ScenarioInfo, which is used at Message creation time to index into the list of Pickles and Pickle Steps. |
Beta Was this translation helpful? Give feedback.
-
|
Implementation Question: I have somewhat liberally used the ITraceListener.WriteToolOutput and WriteTestOutput throughout the code as a mechanism to help me debug/understand the behavior of the Cucumber Messages subsystem as it evolves.
|
Beta Was this translation helpful? Give feedback.
-
|
This is a good question.
But the question remains: then what? I think it would be good to have some sort of diag channel that can be enabled when needed. It's too bad that you have just run into yet another design issue. What would help you right now? Maybe you could add a new method on
The But as mentioned above, currently we don't have any better. Maybe the best would be to be able to route optionally |
Beta Was this translation helpful? Give feedback.
-
|
Implementation Question for Example-driven tests: [Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute()]
[Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute("Log JSON")]
[Microsoft.VisualStudio.TestTools.UnitTesting.TestPropertyAttribute("FeatureTitle", "Cucumber Messages Smoke Outline Test")]
[Microsoft.VisualStudio.TestTools.UnitTesting.TestCategoryAttribute("some-tag")]
[Microsoft.VisualStudio.TestTools.UnitTesting.DataRowAttribute("\"red\"", "9", null)]
[Microsoft.VisualStudio.TestTools.UnitTesting.DataRowAttribute("\"green\"", "11", null)]
[Microsoft.VisualStudio.TestTools.UnitTesting.DataRowAttribute("\"blue\"", "13", null)]
public async System.Threading.Tasks.Task LogJSON(string color, string @pickleId, string[] exampleTags)
{
string[] @__tags = new string[] {
"some-tag"};
if ((exampleTags != null))
{
@__tags = System.Linq.Enumerable.ToArray(System.Linq.Enumerable.Concat(@__tags, exampleTags));
}
string[] tagsOfScenario = @__tags;
System.Collections.Specialized.OrderedDictionary argumentsOfScenario = new System.Collections.Specialized.OrderedDictionary();
argumentsOfScenario.Add("color", color);
string m_pickleId = @pickleId;
global::Reqnroll.CucumberMessages.RuntimeSupport.PickleStepSequence m_pickleStepSequence = testRunner.FeatureContext.FeatureInfo.FeatureCucumberMessages.PickleJar.PickleStepSequenceFor(m_pickleId);
global::Reqnroll.ScenarioInfo scenarioInfo = new global::Reqnroll.ScenarioInfo("Log JSON", null, tagsOfScenario, argumentsOfScenario, featureTags, m_pickleId, m_pickleStepSequence);
#line 3
My question is about naming of these system generated parameter names and how to avoid naming conflicts with what the user may use as a column name in a table. As you can see above, I used the ampersand character at the start of the parameter name (valid C# syntax) in an attempt to avoid name clashes.
|
Beta Was this translation helpful? Give feedback.
-
|
When I needed such thing in the |
Beta Was this translation helpful? Give feedback.
-
|
Resolved: used double underscore prefix. |
Beta Was this translation helpful? Give feedback.
-
|
Another way to go about this would be to ensure that we don't directly use user-supplied strings as part of generated source text. In the example above, perhaps instead of using the text 'color' as the name of the parameter to the method, we use 'color_6549786' where the number suffix is the hash of the provided string. Same would go for using Feature names as class names and Scenario titles as method names. Doing so has the added advantage that those classnames, method names, etc would not clash with Reqnroll names and we could do away with the 'global::' type references. |
Beta Was this translation helpful? Give feedback.
-
|
Design Issue: Support for Parallel Execution of Scenarios and Cross Process Coordination When the upcoming PR is accepted that supports parallel execution of tests at the scenario(aka test case) level, the test execution framework may execute those tests in separate processes (or for the purposes of this discussion, in separate AppDomains). This significantly complicates the design of Messages support. We will need a means to coordinate across these executing processes to coordinate access to the results file and to ensure metadata records such as the StepDefinition messages are written only once. All messages will have to be sent to a central coordinator whether that is one of the processes executing the tests or some other dedicated process. Is there anyway for the user to configure their test fixture's use of process models? If so we might insist that these two features are incompatible(for now). Otherwise does anyone have suggestions on technologies/frameworks to use? |
Beta Was this translation helpful? Give feedback.
-
|
Under .NET Core the whole AppDomain concept was thankfully dropped, but the APIs still exist in .NET Standard and still work in .NET Framework versions. |
Beta Was this translation helpful? Give feedback.
-
|
I've skimmed through the Scenario parallelization code and related documentation. Am I reading it correctly that this will only be supported in two of the three test frameworks? And that it these parallel instances will be running in the same process? (aka, multiple instances of the generated test class all running in a single proc) |
Beta Was this translation helpful? Give feedback.
-
|
Yes, xUnit currenly does not support method-level parallelization and all the other two does in in-process. |
Beta Was this translation helpful? Give feedback.
-
|
There is actually a way to make xUnit create a collection per method by implementing a custom XunitTestFrameworkExecutor. If you override RunTestCases and basically create a new test collection intsance for each test case provided, then they will all run in parallel. See as an example: xunit/xunit#1986 (comment) It won't change the process model too much though. |
Beta Was this translation helpful? Give feedback.
-
|
So, as long as everything is in a single process, the branch I have now should suffice. |
Beta Was this translation helpful? Give feedback.
-
|
Implementation Question: ISkippedStepHandler - there is a separate list of SkippedStepHandlers maintained at the Scenario level; when a step is skipped, the TestExecutionEngine iterates through the list and invokes each handler. Given that this is a mechanism by which Reqnroll could be calling out to user-supplied code, was considering modifying this interface to async/await. But I can find no implementation methods that actually use it (the only references to it are in the tests).
|
Beta Was this translation helpful? Give feedback.
-
|
Design & Implementation Notes on Message properties not populated
|
Beta Was this translation helpful? Give feedback.
-
|
R27 of Cucumber Messages was just released this morning. Here are the release notes: I will be working to adapt to this (adding the TestRunHookStarted and Finished messages). Questions:
So, we have a few options to consider: Let me know your thoughts on direction. |
Beta Was this translation helpful? Give feedback.
-
Over on the Messages github, I asked about extending the enumeration to include Before/After Feature. It sounds like they would not be in favor since their Cucumber implementations don't have Feature surrounding hooks. |
Beta Was this translation helpful? Give feedback.
-
|
I would choose this option:
If later we need to have the feature hooks, we will still have the ID to distinguish, but I would not bother with it now. |
Beta Was this translation helpful? Give feedback.
-
|
OK. When I update the PR to the new version of Messages I will add the Id to the TestRunStarted and Finished messages, and add the TestRunHookStarted/Finished messages (mapping them to BeforeTestRun/AfterTestRun execution events). Please note that accepting this latest release of Messages from the Cucumber repo will need to wait until they also provide a new release of the CCK as the test-suite I've built relies on the CCK to provide explicit examples of conformant messages. |
Beta Was this translation helpful? Give feedback.
-
CCK has been updated and I've adapted those changes in to the validation test suite. |
Beta Was this translation helpful? Give feedback.
-
|
Code Structure/Design Question: in the ExecutionEvent.cs file, I've created new class constructors for HookBindingStartedEvent, HookBindingFinishedEvent, OutputAddedEvent and AttachmentAddedEvent. For each class, the new constructor takes additional parameters, then delegates back to the original constructor. (The additional parameters are required to indicate to the Messages code where in the stream of execution these events belong, ie, to which Feature/Scenario/Step) Perhaps other than in tests, the original constructors are no longer used in the main body of Reqnroll; I've replaced all those invocations with calls to the new (larger) constructors. Should these old constructors be marked as [Obsolete], leave them as-is, or should these be refactored away completely? |
Beta Was this translation helpful? Give feedback.
-
|
Assuming the constructors are all part of the published, public interface, this feels like it should be the other way around: parameters should be added to the new constructor and a backwards-compatible mechanism to emulate the existing constructor. It may be possible to do this with optional parameters, or an overload may be required. I would only suggest marking a constructor as being obsolete if it actively causes a problem to use it. If the code is written in such a way that the old constructors work to support our tests, that implies the extra values are optional and may not be present in all cases, in which case we're fine to leave the constructors there for other user's mocking and testing purposes. If this isn't the case, if we're saying these values are always present once this code-branch is merged, then we should consider whether it's valid for instances of these events to exist in tests without the new fields. |
Beta Was this translation helpful? Give feedback.
-
|
Here is an example: public class HookBindingFinishedEvent : ExecutionEvent
{
public IHookBinding HookBinding { get; }
public TimeSpan Duration { get; }
public IContextManager ContextManager { get; private set; }
public Exception HookException { get; private set; }
public HookBindingFinishedEvent(IHookBinding hookBinding, TimeSpan duration)
{
HookBinding = hookBinding;
Duration = duration;
}
public HookBindingFinishedEvent(IHookBinding hookBinding, TimeSpan duration, IContextManager contextManager, Exception hookException = null) : this(hookBinding, duration)
{
ContextManager = contextManager;
HookException = hookException;
}
}I added the second constructor, which delegates down to the first. In the Reqnroll code-base, all creations of this object use the newer, larger constructor. The question is whether to retain the older, smaller one in case some other code-bases, such as custom Plugins, were perhaps using them. (I can't find any in the Reqnroll solution, but it's possible that someone may have built a private plugin that does) Once merged, Reqnroll has no use for the first constructor; but should we keep them for backward compatibility? |
Beta Was this translation helpful? Give feedback.
-
|
Keep them and mark them obsolete. |
Beta Was this translation helpful? Give feedback.
-
|
Resolved: Marked them as Obsolete. |
Beta Was this translation helpful? Give feedback.
-
Cucumber Messages is a "standard" JSON format that describes the feature files and the test executions. Most Cucumber-family tools can produce Cucumber Messages JSON output so it allows creating reporting and other tools that work with all cucumbers.
The message classes and the JSON schema are described in the GitHub project: https://github.com/cucumber/messages
Reqnroll does not currently produce cucumber messages output. LivingDoc generation and other tooling could benefit from this.
Beta Was this translation helpful? Give feedback.
All reactions