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

Support running tests with limited resources (e.g. browser based) #107

Closed
pawelpabich opened this issue Jun 1, 2014 · 24 comments
Closed

Comments

@pawelpabich
Copy link
Contributor

Hi,

I've been looking for a while for a test framework that would let me run browser based tests in parallel and it looks like xUnit has most of the required features. I blogged a detailed analysis here (http://www.pabich.eu/2014/06/how-to-run-selenium-tests-in-parallel.html) but I will include all important information here so the discussion is self-contained. The purpose of this discussion is to see whether those missing features fit into the vision of the xUnit project and if so, how they can be implemented. Personally I believe that the famous extensibility capabilites of xUnit could help here a lot (e.g. custom test execution pipeline via a custom implementation of XUnitTestCase). And having one, instead of many frameworks, has its obvious adventages.

I forked xUnit https://github.com/pawelpabich/xunit to see if I can fill the gaps. This was easier than I thought it would be thanks to great condition the xUnit codebase is in. The whole excercise is more a Proof of Concept then a fully blown implementation but it works and it is used successfully in production.

I split the missing features into 3 groups:

  • must have (without them browser based tests won't be possible with xUnit),
  • should have (very,very usefull and it will make life much easier but not deal breakers)
  • nice to have (there is no rush to add those, but eventually it would good to have them)

Must haves

Report test result back to the test before it gets disposed

Tests need to know when they fail so a screenshot of the web browser can be taken. Current solution checks whether the test class implements certain interfaces, e.g. INeedToKnowTestFailure.

Execute the whole test as a single Task

Current design splits the execution pipline into mutliple tasks which means that Dispose method of an already executed test might be run with some delay which means more browser instances will be required than what MaxParallelThreads would suggest. In my tests I very often ended up with 2 or 3 x MaxParallelThreads number of browsers. Current solution is to execute the whole pipleline as a single Task

Should haves

Make unit of work more granular

At the moment the smalles unit of work for parallel processing is class. For long running tests this is not optimal as a class with a few long running tests can significantly slow down the whole build. Current solution introduces a new, per method, collection bahaviour.

Allow to order test collections

Assuming that we can have a test collection per test methed then the next logical step to minimize the exectuion time would be to have ability to order test collections to make sure the failed tests or/and tests with longest execution time are run first. Current solution is based on the existing extensibilty point for ordering tests within a test collection.

Nice to haves

Allow to perform global initalization

At the moment global initialization needs to happen in the instance constructor with global lock.

Allow to perform global clean up

Current workaround relies on subscription to AppDomain.Unload event which works most of the time but not always.

I really hope we can make it work :). Thoughts?

@jorgef
Copy link

jorgef commented Jun 20, 2014

+1

I think it is important to have those features, or at least to add some extensibility points so it is easier to change its default behaviour without having to branch it.

@HEskandari
Copy link

+1 as well, considering all other unit testing frameworks look archaic by not supporting async / task and running tests in parallel, this would give us xUnit users another reason to look nowhere else, making this a de facto unit and functional testing framework in .NET.

@ghost
Copy link

ghost commented Jun 20, 2014

+1

It would be great to have those features in xUnit especially now, when we cannot use any other framework for proper parallel tests run.

@robdmoore
Copy link

+1

1 similar comment
@mavadat
Copy link

mavadat commented Jun 24, 2014

+1

@bradwilson
Copy link
Member

All of these requirements tell me you're going to want to implement your own test framework (at least ITestFrameworkExecutor). You should be able to leverage existing code in base classes, especially since the big runner split out that started in Beta 2 and continues in Beta 3.

@pawelpabich
Copy link
Contributor Author

Brad,

I will have a look what I can do with this interface. I will be on leave for couple of weeks so it might take me some time.

@pawelpabich
Copy link
Contributor Author

@bradwilson one more question, I understand that the whole package of changes sounds like a new framework but what is your thinking about features that are extensions of existing features (e.g test collection per method) or conceptually are based on existing concepts (TestCollectionOrderer which is based on TestOrderer)?

@saugustine
Copy link

+1

1 similar comment
@KungRaseri
Copy link

+1

@pawelpabich
Copy link
Contributor Author

Hi @bradwilson,

I had a look what it would take to create a custom test framework based on xunit that would help me solve my problem. This is that what I ended up with pawelpabich@0494e78.

I'm sure I've missed a few things but the idea was to subclass as much as possible and avoid copying and pasting code and then plug my custom test framework via TestFrameworkAttribute. The end result is that It looks like without additional abstractions in the framework I won't be able to achieve what I'm looking for. I would love to be proven wrong :).

Taking this all into account, would you guys be willing to:

  • introduce TestCollectionOrderer which conceptually is based on TestCaseOrderer but deals with test collections, this will let me execute the longest and/or failed tests first
  • introduce PreMethodTestCollection so tests from the same class can be run in parallel
  • make TestInvoker.RunAsync virutal so I could notify the test about its result and execute constructor, test and its dispose method as a single task

As far as I can see they can be implemented without introducing any breaking changes. I'm more than happy to provide pull requests for them all of them.

Thoughts?

@bradwilson
Copy link
Member

We've opened #174 to track the test collection orderer.

As for PreMethodTestCollection, we have solidified on the design of the test collection being the boundary for parallelization. Allowing test methods in the same class to be in different test collections is a major breaking change with regard to fixtures.

We could do your third request (virtualize RunAsync), but it seems like the only reason you'd need that would be to parallelize test methods. Is there any other reason to make that virtual?

@pawelpabich
Copy link
Contributor Author

Great :).

  1. Would you like me to create a PR for Add TestCollectionOrderer #174 ?
  2. I need to override RunAsync so constructor, the test method and the dispose method are all called as part of the same task. Do you see any other way of achieving this?
  3. Re PerMethodTestCollection, would introduction of the new level be a breaking change? The default could still stay the way it is (PerClass). Unless you are talking about IClassFixture and ICollectionFixture. In such a case they would have to be thread safe but that would be expected when someone wants to change PerClass to PerMethod. Just curious, because I am running a custom build of xunit at the moment and I'm not seeing any issues with PerMethod but it looks like I might be just lucky.

@bradwilson bradwilson modified the milestone: post-2.0 Oct 4, 2014
@jfloodnet
Copy link

+1

@bradwilson bradwilson removed this from the later milestone Jul 5, 2015
@RichiCoder1
Copy link

👍 Just running into this. Deterministic disposal and any easy way to globally setup and teardown state are my biggest issues.

@bradwilson
Copy link
Member

The demands at the top of this issue are not things we are planning to deal with; our focus with xUnit.net is to keep the core focused on TDD, with extensibility to let other people adapt it to their needs. Our recommendation is still to write your own test framework (implementation of ITestFramework). There is an example of this here: https://github.com/xunit/samples.xunit/tree/master/ObservationExample

@RichiCoder1
Copy link

That's how I ended up solving it. Abstracted away TestFramework and TestExecutor to handle IIS Express spin up. If anyone else in this thread is interested in building a general purpose selenium test framework on top of xUnit using the Xunit.Sdk, let me know.

@pawelpabich
Copy link
Contributor Author

@RichiCoder1 would love to have a look

@jfloodnet
Copy link

@pawelpabich @RichiCoder1 me too - at this stage I've just subclassed the XUnitTestCaseRunner.

@RichiCoder1
Copy link

Reading over the goals you were aiming for @pawelpabich, I think we had slightly different use scenarios. I was most interested in per-assembly Setup and Teardown, and I prefer the class-as-collection and per-collection behavior. That being said, I'm cleaning up and de-companyizing my code and will push it out to GitHub in a little. After that, we can make a few issues and see about building an all-encompassing, nuget-pluggable solution. Sound good?

@pawelpabich
Copy link
Contributor Author

Sounds like a good start @RichiCoder1 :)

@RichiCoder1
Copy link

Dropped a very basic framework here: https://github.com/RichiCoder1/Browser.xUnit

Going to add the custom setup and teardown extensibility points I added elsewhere here in a little. Feel free to create issues and start discussions. My goals are very simplistic and I want to make sure we come up with a good solution :)

Edit: And it is using Dnx. Ensure you have dnx installed and run dnvm upgrade and well as dnu restore on the repository before building.

@pawelpabich
Copy link
Contributor Author

@RichiCoder1 looks good, thanks!

@bradwilson
Copy link
Member

Closing this, as it's something that should be done externally.

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

No branches or pull requests

10 participants