Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Feature Tag called parallelize #746

Merged
merged 18 commits into from Jan 31, 2017

Conversation

tmulkern
Copy link
Contributor

@tmulkern tmulkern commented Nov 24, 2016

Added a tag called parallelize which which is tag that works at a Feature level. It will generate code for the Feature to run explicitly in parallel for Munit3, Nunit3 and Xunit2.

This will allow for Mixing for Features that can be run in Parallel with ones that cannot, it will also add support for enabling MUnit3 generated tests to run in parallel because unlike Nunit3 use of ParallelScope, MUnit3 must explicitly set which TestFixtures can run in parallel

Adds ParallelizableAttribute in case of Nunit3 and MbUnit3 generators
and TestCollection with Unique Name (Guid) for Xunit2 generator when
using parrallelize tag
@SabotageAndi
Copy link
Contributor

Thanks for the PR. I think this is a good idea to have such a feature, but I am not sure about the name.
Perhaps we should but such special tags in a "namespace" so that if someone is already using this tag, has with an update new, unexpected behaviour.
I was thinking in that case about @specflow:parallelize. @gasparnagy any thoughts on this?

@tmulkern Could you fix the parrallelize- typos (double r) in the code before we have a depper look at it? Somehow it iritates me when I am reading the code. Thanks!

@tmulkern
Copy link
Contributor Author

@SabotageAndi Thanks for the feedback, I know what you mean about typos, will fix them up

@tmulkern
Copy link
Contributor Author

@SabotageAndi I also agree with the namespacing

Change parrallel to parallel
@tmulkern
Copy link
Contributor Author

@SabotageAndi I have fixed the spelling mistake

@SabotageAndi
Copy link
Contributor

Thanks! Looks good for me.
@gasparnagy : Your 2 cents? Can we merge it?

@tmulkern
Copy link
Contributor Author

@SabotageAndi will I add the namespacing as you suggested?

@tmulkern
Copy link
Contributor Author

@gasparnagy bump

@godrose
Copy link

godrose commented Nov 30, 2016 via email

@tmulkern
Copy link
Contributor Author

@godrose I am just waiting for the OK on this PR and get it merged. Hopefully it'll make the 2.2 release

@gasparnagy
Copy link
Contributor

@tmulkern @godrose @SabotageAndi I generally think that this is something that would be useful for many people, but I adding predefined English tag names is problematic. The problem is that while many people use SpecFlow with English gherkin files, there are a lot of projects that use these scenarios to have a better conversation with their stakeholders in a non-English project. And you should also think about languages that do not even use Latin letters, like Russian or Chinese.
Therefore our goal was to leave the choice for choosing the tag that is fitting to their communication model, and somehow let them map it to concrete functionality. (As you might have spotted out, there is already one that was predefined, the @ignore tag, but this is rather a bad example from this perspective.)

I am not saying that we should turn this idea down, but we should think about what other options we have that works better for non-English gherkin files. (Of course we could turn this into a plugin, so that it is not part of the core and people can either use it as it is or make their own version with their own tag name, but the problem we solve here is not plugin-like, as the parallelization is part of the core anyway.)

Also this way of attribution represents a kind of opt-in model, so you need to mark those features/scenarios that you want to run parallel, where in many cases an opt-out model works better.

Could you maybe describe the situation where you needed to separate the parallel and non-parallel running tests? Maybe seeing a concrete example might also help to see what would be the best way to address this problem.

@tmulkern
Copy link
Contributor Author

tmulkern commented Dec 1, 2016

@gasparnagy Thanks for getting back to me

  • I take your point on the different languages, but as you pointed out the ignore tag is already there, since parallelization is part of the core, I didn't think this would be an issue having this tag in English

  • I can easily change this to be an opt-out model with a @DoNotRunInParallel tag instead? Then create an assembly level attribute that can turn on parallelization that can be used for all frameworks?

  • I don't think the example will help, I think is is a simple enough to understand without an example. I am just trying to emulate in Gherkin which is already functionality of the frameworks mentioned.

@tmulkern
Copy link
Contributor Author

tmulkern commented Dec 1, 2016

@gasparnagy
Maybe even better would be an assembly level attribute that specifies the opt-out tag in the stake holders language? Something like [assembly: SpecflowParallelize(doNotRunInParallelTag:"DoNotRunInParallel")]

That way it solves the language issue because you have to explicitly set the tag name

@tmulkern
Copy link
Contributor Author

tmulkern commented Dec 1, 2016

@gasparnagy Or if not an assembly attribute, then a conflict section element in the app.config?

@SabotageAndi
Copy link
Contributor

@gasparnagy I think making parallelization opt-out will break a lot of existing tests, when SpecFlow gets updated there.
First the users get propably a lot of errors because they were using ScenarioContext.Current which is then not more possible.
Second, the chance that the tests and the tested software are not written for parallel tests is high. This will result in conflicts.

I have the opinion, that how the scenarios are executed is a job of the test runner and not of SpecFlow. If it runs the Scenarios per default in parallel, thats fine and the user knows that. If it runs one Scenario at a time, it is also fine.
AFAIK enabling parallel execution is opt-in on NUnit, so why should me make an opt-out of that. This is a surprise behavior difference for the user.

@tmulkern
Assembly attributes will not work, because we need the information at generation time. Config- section would be fine for me.
Addtionally we could provide standard values for the different languages.

@tmulkern
Copy link
Contributor Author

tmulkern commented Dec 1, 2016

@SabotageAndi my idea was not to enable parallelization by default and then selectively opt-out the test cases, but instead have a configuration entry that would switch on the generation of parallelization code (for the frameworks mentioned above), with an opt-out tag that is specified in the config that will disable the generation of parallelization code for that 'tagged' feature.

Example (open to suggestions for the attribute names):

<generator allowDebugGeneratedFiles="false" allowRowTests="true" generateAsyncTests="false" generateParallelizationCode="true" doNotRunInParallelTag="DoNotRunInParallel" />

In regards to the standard values for the different languages I would not be in favor as it is much easier for the user to specify the tag in the config. That way there would not be the additional overhead of standard values for n languages and n charsets

The only way that I know of enabling parallelization with out generating any additional code right now (correct me if I am wrong) is by using Nunit3 and adding the an assembly level attribute as stated in the wiki https://github.com/techtalk/SpecFlow/wiki/Parallel-Execution#notes-for-nunit-v3-support.

To enable parallelization in XUnit2 requires adding the CollectionAttribute and for Mbunit3 requires adding the ParallelizableAttribute

TBH I originally started this PR to enable parallelization for Mbunit, but I decided to come up with a way to implement it for all the Frameworks that can support parallelization

@SabotageAndi @gasparnagy If you are ok with my suggestions (from this comment) I will make changes to PR to reflect the conversation.

@gasparnagy
Copy link
Contributor

@tmulkern @SabotageAndi thx for the feedback.

@tmulkern I don't agree with you about the value of an example. Examples are the driving forces in BDD and help us to implement something that is really valuable for our users and not just features that we have to maintain. I have also done this mistake and I could list several features in SpecFlow that are actually not used by anyone, but causing troubles, like the BeforeScenarioBlock hook. But let's get back to our topic.

I can imagine the following scenario (this is an example):

  • you have an existing project with many scenarios, execution time is too long
  • you switch to xUnit 2 and everything starts to break
  • you switch off parallel execution in xUnit, because you have realized that your project is not ready yet for parallel
  • you work on making the project parallelizable, you fix a large part of it, so you could benefit from the faster execution, but there is a small portion that depends on an external dependency that you cannot test parallel
  • you re-enable parallel in xUnit but you would like to exclude those scenarios that need that external dependency

At this point this example would lead to the opt-out model, but actually I would not even need a @parallel attribute here, because my scenarios that have been tagged with the @myspecialexternaldep are the ones that I would like to exclude (for now at least).

So if we start from this, we should rather implement this feature in a way, that you can specify a list of tags in the specflow configuration and if any of them is used in a scenario, we should disable the parallel execution.

Maybe from another example we should get to another solution, this is why I would be really interested in what was the example (in the level I was listing it), that have made you starting this PR. I would be also interested in yours, of course, @SabotageAndi.

@tmulkern
Copy link
Contributor Author

tmulkern commented Dec 2, 2016

@gasparnagy I think I mistook your request for an example, I though you were looking for some examples in Gherkin, which would have a bit of a bother as the use cases I have in Gherkin are subject to NDA etc. But the format you have described, I am happy to oblige, sorry about the confusion ;-)

As it happens the scenario you described is exactly the scenario I am having with MBunit. Right now it's an all or nothing approach with running in Features in parallel, so opting-out tags would be fine with me. The reason why I started with an opt-in model is that it was simpler to implement, but after your feedback I see that the opt-out model, with user specified tags is a much more useful model.

I'll get started on writing the code for this, thanks.

* Added changes to load opt-out tags from config
* Added changes to load global flag from config to generate parallel
code
* Renamed ParallelizeDecorator to DoNotParallelizeDecorator
* Added Unit Tests
@tmulkern
Copy link
Contributor Author

tmulkern commented Dec 4, 2016

Ok, made the changes to the code, this is a example of what the config would look like

<generator allowDebugGeneratedFiles="false"
                           generateParallelCodeForFeatures="true">
                    <ignoreParallelTags>
                        <tag value="mySpecialTag1"/>
                        <tag value="mySpecialTag2"/>
                    </ignoreParallelTags>
</generator>

@tmulkern
Copy link
Contributor Author

tmulkern commented Dec 6, 2016

@gasparnagy @SabotageAndi Does the updated code look good?

@SabotageAndi
Copy link
Contributor

@gasparnagy? From the code perspective it looks good.

@tmulkern
Copy link
Contributor Author

@gasparnagy All those changes are in

@gasparnagy
Copy link
Contributor

@tmulkern thx for the fast response. Somehow the line endings in the DefaultDependencyProvider.cs were changed in your one but last commit. It would be great to keep a proper history for this file, because many changes touch it. Could you somehow fix this and revert the line endings to what it was?

@tmulkern
Copy link
Contributor Author

@gasparnagy I didn't realise that, must be because I edited that file using GitHub's online tools. I must remember to do all my file changes on my computer from now on 😉.

I'll fix the file as soon as I can

@tmulkern
Copy link
Contributor Author

@gasparnagy Ok, changes in

@gasparnagy
Copy link
Contributor

@tmulkern Thx for the contribution! I will merge it now.

@gasparnagy gasparnagy merged commit f3ee6ca into SpecFlowOSS:master Jan 31, 2017
@tmulkern
Copy link
Contributor Author

Great stuff, thanks

@tmulkern tmulkern deleted the tag_parallelize branch February 1, 2017 09:02
@tmulkern
Copy link
Contributor Author

tmulkern commented Feb 3, 2017

@godrose pull request is merged, just to give you a heads up

@brettveenstra
Copy link

in the CI build (ci583), when I turn on parallelization of feature files, the NUnit parallelization values use the Feature file names instead of values like ParallelScope.None, or ParallelScope.Fixtures, etc.

@SabotageAndi
Copy link
Contributor

@brettveenstra Could you please open an issue with this problem and attach a small sample where we can see the failure?
Thanks!

@liamharries
Copy link

Sorry if this is the wrong place to post this, please redirect me if needed.
I'm trying to exclude a test from parallel exection and having some difficulty.
I've included the generator as mentioned into my app.config and applied the tag to my scenario

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <configSections>
    <section name="specFlow" type="TechTalk.SpecFlow.Configuration.ConfigurationSectionHandler, TechTalk.SpecFlow"/>
  </configSections>
  <appSettings>
    ...
  </appSettings>
  <specFlow>
    <!-- For additional details on SpecFlow configuration options see http://go.specflow.org/doc-config -->
    <unitTestProvider name="SpecRun"/>
    <plugins>
      <add name="SpecRun"/>
    </plugins>
    <generator allowDebugGeneratedFiles="false"
               markFeaturesParallelizable="false">
      <skipParallelizableMarkerForTags>
        <tag value="Sequential"/>
        <tag value="sequential"/>
      </skipParallelizableMarkerForTags>
    </generator>
  </specFlow>
  <startup>
    <supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.5.2"/>
  </startup>
</configuration>

The test however still runs in parallel :(

Does anyone know how i can get my test to be excluded from parallel execution?

Below is my test

@Authentication
	Feature: Authentication
	So that only privileged users may access the system
	As the product owner
	I want the product to be authenticated
	
@AllStandard
Scenario Outline: Login
	Given I am at the <page1> page
	When I provide a <username> username and a <password> password
	Then I am at the <page2> page
	And unsuccessful login attempt pop-up is <visibility>

	Examples: 
	| description       | page1 | username | password | page2    | visibility |
	| valid credentials | login | true     | true     | Services | false      |
	| invalid username  | login | false    | true     | login    | true       |
	
@AllStandard
@Sequential
Scenario Outline: LoginSequential
	Given I am at the <page1> page
	When I provide a <username> username and a <password> password
	Then I am at the <page2> page
	And unsuccessful login attempt pop-up is <visibility>

	Examples: 
	| description       | page1 | username | password | page2    | visibility |
	| invalid password  | login | true     | false    | login    | true       |

And my default.srprofile

<?xml version="1.0" encoding="utf-8"?>
<TestProfile xmlns="http://www.specflow.org/schemas/plus/TestProfile/1.5">
  <Settings projectName="iPipelineSpecTests" projectId="{5f94e355-e6ef-4c08-9807-0b89016244a7}" />
  <Execution stopAfterFailures="0"
             testThreadCount="8"
             testSchedulingMode="Random"
             apartmentState="MTA"
             retryFor="Failing"
             retryCount="0"/>
  <TestAssemblyPaths>
    <TestAssemblyPath>iPipelineSpecTests.dll</TestAssemblyPath>
  </TestAssemblyPaths>

  <Targets>

    <Target name="IE_11_Windows10">
      <Filter>@AllStandard | @Desktop_All | @IE_All | @IE_11_Windows10</Filter>
      <DeploymentTransformationSteps>
        <RelocateConfigurationFile target="Thread{TestThreadId}config.config"/>
        <ConfigFileTransformation configFile="App.config" >
          <Transformation>
            <![CDATA[<?xml version="1.0" encoding="utf-8"?>
							<configuration xmlns:xdt="http://schemas.microsoft.com/XML-Document-Transform">
                              <appSettings>
                                <add key="browser" value="internet explorer" xdt:Locator="Match(key)" xdt:Transform="SetAttributes(value)" />
                                <add key="version" value="11" xdt:Locator="Match(key)" xdt:Transform="SetAttributes(value)" />
                                <add key="platform" value="Windows 10" xdt:Locator="Match(key)" xdt:Transform="SetAttributes(value)" />
                              </appSettings>
							</configuration>
						]]>
          </Transformation>
        </ConfigFileTransformation>
      </DeploymentTransformationSteps>
    </Target>

         ...

  </Targets>
</TestProfile>

@tmulkern tmulkern restored the tag_parallelize branch February 4, 2018 15:14
epresi pushed a commit that referenced this pull request Jun 14, 2021
* Remove ParallelizeDecorator and related unit tests.

* Remove IUnitTestGeneratorProvider.SetTestClassParallelize method.

* Remove Parallelizable attribute from NUnit3TestGeneratorProvider.

* Remove two generator configuration items related to the ParallelizableDecorator.

* Update changelog.
Code-Grump pushed a commit to Code-Grump/SpecFlow that referenced this pull request Mar 29, 2023
…S#2444)

* Remove ParallelizeDecorator and related unit tests.

* Remove IUnitTestGeneratorProvider.SetTestClassParallelize method.

* Remove Parallelizable attribute from NUnit3TestGeneratorProvider.

* Remove two generator configuration items related to the ParallelizableDecorator.

* Update changelog.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants