reqnroll / Reqnroll

Open-source Cucumber-style BDD test automation framework for .NET.
https://reqnroll.net
BSD 3-Clause "New" or "Revised" License
310 stars 32 forks source link

The @ignore attribute is not inherited to the scenarios from rule #111

Closed gasparnagy closed 2 months ago

gasparnagy commented 2 months ago

Reqnroll Version

1.0.1

Which test runner are you using?

xUnit

Test Runner Version Number

2.7.0

.NET Implementation

.NET 8.0

Test Execution Method

Visual Studio Test Explorer

Content of reqnroll.json configuration file

No response

Issue Description

In case the @ignore is placed on the Rule keyword it is not effective.

Steps to Reproduce

Have a feature file like:

Feature: F1

@ignore
Rule: R1

Scenario: S1
  When something

Run the tests: S1 is not ignored.

Adding the @ignore directly to the scenario works.

Link to Repro Project

No response

clrudolphi commented 2 months ago

Let's confirm expected behavior:

gasparnagy commented 2 months ago

@clrudolphi I think for other tags it works in general: this test verifies that for scenario outlines. So the only problem is with the @ignore tag (I think).

For this we would need to extend the sample feature (with and ignored rule) and the assertion block here: https://github.com/reqnroll/Reqnroll/blob/main/Tests/Reqnroll.SystemTests/Generation/GenerationTestBase.cs#L129

clrudolphi commented 2 months ago

@gasparnagy I could use some guidance. I have added a confirming test as you suggested. It currently fails (as expected). The generated unit test code is confusing me a bit, Here is the generated test method:

        [Xunit.SkippableFactAttribute(DisplayName="Rule ignored scenario")]
        [Xunit.TraitAttribute("FeatureTitle", "Sample Feature")]
        [Xunit.TraitAttribute("Description", "Rule ignored scenario")]
        public async System.Threading.Tasks.Task RuleIgnoredScenario()
        {
            string[] tagsOfScenario = ((string[])(null));
            System.Collections.Specialized.OrderedDictionary argumentsOfScenario = new System.Collections.Specialized.OrderedDictionary();
            global::Reqnroll.ScenarioInfo scenarioInfo = new global::Reqnroll.ScenarioInfo("Rule ignored scenario", null, tagsOfScenario, argumentsOfScenario, TagHelper.CombineTags(featureTags, new string[] {
                            "ignore"}));
#line 41
 this.ScenarioInitialize(scenarioInfo);
#line hidden
            if ((global::Reqnroll.TagHelper.ContainsIgnoreTag(tagsOfScenario) || global::Reqnroll.TagHelper.ContainsIgnoreTag(featureTags)))
            {
                testRunner.SkipScenario();
            }
            else
            {
                await this.ScenarioStartAsync();
#line 42
 await testRunner.WhenAsync("the step passes", ((string)(null)), ((global::Reqnroll.Table)(null)), "When ");
#line hidden
            }
            await this.ScenarioCleanupAsync();
        }

The value of "ignore" is concatenated with the feature-tags and then stored in the CombinedTags field of the ScenarioInfo during the call to the constructor of the ScenarioInfo. But the check for whether to call testRunner.SkipScenario(), in the subsequent lines, for Ignore doesn't look at the tags on the SI, it only looks at the test method variable, tagsOfScenario, and the test Class field, featureTags. Should this if() statement be retrieving the scenarioInfo.CombinedTags Property and using that value in the call to TagHelper.ContainsIgnoreTag()?

Secondly, the XUnit.SkippableFactAttribute does not include a Skip property like the other 'ignored' test in the feature-file:

        [Xunit.SkippableFactAttribute(DisplayName="Ignored scenario", Skip="Ignored")]

Is the presence of that value required in order for the test to be ignored or does the method invocation of SkipScenario() cause the test to be ignored?

gasparnagy commented 2 months ago

@clrudolphi regarding my memories ignoring works in the following way: For scenarios the @ignore should be handled with attributes (Skip="Ignored" or [Ignore]), but if a scenario outline example set it ignored, that we cannot ignore with attribute, but we need to dynamically ignore the test using the if (in that case the attributes of the example are passed in as a parameter). Maybe this is why the generated code looks so strange.

But you should move the @ignore tag directly to the scenario and check the difference.

Was it failing only for xUnit or also for the others? Is the generated code similar for the others too?

clrudolphi commented 2 months ago

@gasparnagy I now have something working. Pls review this and let me know your thoughts. I am somewhat cautious of unintended side effects. The test I have defined is this:

            @ignore
            Rule: Scenario in this Rule should be Ignored
             Scenario: Rule ignored scenario
             When the step passes

The System test condition is added to the section that checks for Ignored tests:

        _vsTestExecutionDriver.LastTestExecutionResult.LeafTestResults
                              .Should().ContainSingle(tr => tr.TestName.StartsWith("Rule ignored")) 
                              .Which.Outcome.Should().Be(expectedIgnoredOutcome);

I've modified the code generator so that the dynamic check for ignore now looks like this:

 if ((global::Reqnroll.TagHelper.ContainsIgnoreTag(scenarioInfo.CombinedTags) || global::Reqnroll.TagHelper.ContainsIgnoreTag(featureTags)))
            {
                testRunner.SkipScenario();
            }

This passes with all three test frameworks. A - should the code generator be further changed to simply check for ignore tags in ONLY the scenarioInfo.CombinedTags property (and remove the OR condition with the feature tags)? My reasoning here is that the CombinedTags property already includes the feature tags from when the ScenarioInfo was constructed - is that correct? B - should any other modification be made? What am I missing?

Edited to add link to a draft PR #113

gasparnagy commented 2 months ago

@gasparnagy Answered as a review at the PR: I think this is good as it is.

gasparnagy commented 2 months ago

Fixed by #113