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

Order of Fixture Execution #345

Closed
CharliePoole opened this issue Nov 8, 2014 · 39 comments · Fixed by #2365
Closed

Order of Fixture Execution #345

CharliePoole opened this issue Nov 8, 2014 · 39 comments · Fixed by #2365

Comments

@CharliePoole
Copy link
Contributor

In additon to ordering test cases as envisioned by #170, we will probably want to provide for ordering of test fixtures. See discussion at https://groups.google.com/forum/#!topic/nunit-discuss/u2wKj0gTSko

@CharliePoole CharliePoole added this to the Future milestone Feb 21, 2015
@Sebazzz
Copy link

Sebazzz commented Mar 19, 2015

I'd vote for this. This is very useful for integration tests and actually the only reason I use MSTest for that.

@CharliePoole CharliePoole modified the milestones: Future, Ideas Dec 4, 2015
@CharliePoole CharliePoole modified the milestone: Ideas Jun 24, 2016
@NecroFilja
Copy link

Are there any workarounds?
Generally what is default sort order of testfixtures/suites and any way to inject inside of it?

@CharliePoole
Copy link
Contributor Author

In the past they were sorted alphabetically. You can experiment to see if that's still true since its really just a side effect. You have to take the full namespace into account.

@CharliePoole
Copy link
Contributor Author

If course, if you're a developer you can always volunteer to implement this, under guidance if needed.

@Sebazzz
Copy link

Sebazzz commented Jul 4, 2016

@NecroFilja You could add set the Order property on an ITest by creating a attribute implementing IBeforeTest. However, the ordering is only applied within namespace. You'd need to implement a custom ITestAssemblyBuilder, however, the default implementation can't currently bey overwritten. I wrote a short piece about implementing test fixture ordering.

@CharliePoole
Copy link
Contributor Author

@Sebazzz Wow! All that work and no contrib to NUnit... too bad. :-(

Seriously, ordering of tests is always within the parent test in NUnit and the namespace represents a parent test. It really has to be that way, since you can associate OneTimeSetUp methods with namespaces using SetUpFixtures.

@CharliePoole CharliePoole added this to the Backlog milestone Jul 4, 2016
@CharliePoole
Copy link
Contributor Author

I've gone ahead and promoted this from an idea to a feature and added it to our backlog. In our parlance, that means it's something we would like to get done, whereas an idea is just something we are talking about.

No guarantees about when we get to it, but it's also labeled as an easy fix if somebody wants to do it. What's needed to implement:

  • Allow the Order attribute on classes
  • Nothing else as far as I can see!

The code at https://github.com/nunit/nunit/blob/master/src/NUnitFramework/framework/Internal/Execution/CompositeWorkItem.cs#L248 that creates work items already looks at fixtures, so it should take care of things automatically if it finds the property has been set.

One issue I forsee is that you could have multiple SetUpFixtures in the same namespace. If they both had Order attributes, I think the last one that got there would win. It may not be that big a deal.

@yauhen
Copy link

yauhen commented Jul 4, 2016

In the past they were sorted alphabetically.

@CharliePoole as far as I could see on current moment there is only GetTypes order of which is generally unpredictable (of course we now that in reality it return alpha sorted)

CompositeWorkItem.cs#L248

@CharliePoole Thanks for pointing out. Just found myself. For me it looks like it is just enough to removed limitation for Order attribute (only allow to be used for methods). And due of this code it will works as it should (as temporary solution). Or are there any conceptual pitfalls (generally why this limit was set).

@Sebazzz I will try to read more carefully, initially looks a little bit over-complicated.

@CharliePoole
Copy link
Contributor Author

The limitation to methods is merely because we received a request to have OrderAttribute on methods, nothing more than that. I'd be glad to have a PR for this change from someone who wants to get started on NUnit with a rather easy task.

@Sebazzz
Copy link

Sebazzz commented Jul 4, 2016

@CharliePoole Happy to create a pull request to allow a custom ITestAssemblyBuilder implementation. I wasn't sure if you'd want something like that in NUnit.

@yauhen Yes, it is a little bit complicated but it is built in mind with what you can achieve with MSTest ordered tests today.

@CharliePoole
Copy link
Contributor Author

@Sebazzz Of course I was talking about the simple implementation. I use "easyfix" as bait to attract new contributors. :-)

I haven't looked at MsTest for a while. Ordered tests seem like the outgrowth of what used to be called a test list. Is that how you see it? That strikes me as something a bit more transient than the kind of ordering we are talking about with OrderAttribute. That is, OrderAttribute would be used for tests within a fixture (or fixtures in a namespace) that have some intrinsic reason for running ahead of the rest in a certain order all the time. OTOH, a more global ordering such as you describe would be very useful if I wanted to run all recent failures first or run the tests that were touched by a recent edit.

The idea of a custom TestAssemblyBuilder is certainly interesting. I'd have a few questions about the implementation, but if you have time to do a PR we can talk about it there.

The type of ordering you are doing, however, seems to go against the direction being taken by NUnit, which is that the order tests show up in the tree should have little connection with the order in which they are executed. There are actually three orderings here:

  1. The order in which tests appear in the object tree and the XML representation. This is intended to be meaningless, although some people have been known to rely on it. Of course, the nesting itself is not meaningless, just the ordering.
  2. The display order in a particular runner. Many runners are lazy and just display things as they get them. Others will sort them in the order they want to see them displayed.
  3. The execution order. This is a specific ordering performed on WorkItems that are constructed to represent tests. Currently these map to tests one-to-one, but that won't always be so. Other things being equal, this will be the same as (1) but that's not to be relied upon. Of course, in a parallel environment, ordering becomes a bit looser and only refers to the order in which tests are put into an execution queue.

So, any attempt to control ordering really should be dealing with (3) not (1). In fact, changing (1) means changing the representation of the tests in the XML and so should be avoided. Of course, that means that implementing lists as you have done would require changing both (2) and (3), which is a bit of a pain. Maybe the Gui project needs a fourth way to display tests?

@CharliePoole CharliePoole removed this from the Backlog milestone Jul 25, 2016
@CharliePoole
Copy link
Contributor Author

As commented earlier. The "easy fix" for this is to allow the Order attribibute on classes. I think that ordering of the fixtures will "just happen" as a result. Once we get that working, we can investigate the corner cases like multiple setup fixtures on a namespace.

@Sebazzz
Copy link

Sebazzz commented Oct 13, 2016

Building my test ordering library I've found a few things:

  • If you want to run things in order, you need to disable parallelism for the tests which need to run in order
  • Despite this, test methods will still run out-of-order as opposed to the fixtures which declare them. The reason is that NUnit doesn't immediately execute the tests as soon as they are discovered, but queues them instead, unless TestExecutionContext.IsSingleThreaded is set to true.

@CharliePoole
Copy link
Contributor Author

@Sebazzz Yes, both points are covered by my earlier comment here, although perhaps not made clear enough.

"Ordering" in NUnit is viewed as an approximate thing and can't be used for situations in which one test must complete (or perhaps succeed) before another starts. We view that as a kind of "dependency" rather than an "ordering". This has been discussed in many of the threads around method ordering and this issue - about fixture ordering - will fall into the same area.

A loose sort of ordering is useful for many purposes, but not for others. It sounds like you want ordering that is much stronger than we have implemented so far. For that, you have to wait for implementation of the dependency feature we have talked about.

That said, we could make the present OrderAttribute a bit stronger. I'm open to suggestions for small incremental changes that don't try to turn it into a DependsOnAttribute.

I do realize that many people talk about dependency and ordering as approximately the same thing. I don't agree with that. I think there are several "flavors" of dependency and I want us to deal with all of them. There are also various things that could be meant by ordering. In our case it means: "Queue the test in this order. " For some configurations, that's equivalent to "Execute the tests one after the other, without overlap" but for others it isn't.

I don't understand your statement "test methods will still run out-of-order as opposed to the fixtures which declare them." Could you expand on what you mean?

@CharliePoole
Copy link
Contributor Author

Responding here to an offline query about working on this issue... Tom, please post here with your github id so we an give you credit for working on it...

There has been a lot of discussion about both ordering and the much more complex topic of test dependency. This issue, marked as an easy fix, is just about ordering... of test fixtures in this case.

I believe that it is merely necessary to allow the attribute to appear on a class and operate on the fixture in order for ordering to work with test fixtures. The code for ordering work items operates on tests of all levels, without regard to whether the test is a test suite or a test case. Of course, it's possible that some complications will come up when actually doing the work, but we can discuss them here.

Note that the solution to this issue is expected to share all the limitations of ordering already noted for test methods. It only impacts the order in which tests are initiated. It only applies to the relative ordering of tests within the same container - namespace in this case - and so on... That's why it's an easy fix. 😄

@jnm2
Copy link
Contributor

jnm2 commented Jun 26, 2017

@CharliePoole is this issue unofficially assigned right now?

@CharliePoole
Copy link
Contributor Author

@jnm2 Nope. I don't even recognize the concept. 😄

Note however that in spite of all the discussion about dependencies, this is only about applying OrderAttribute to fixtures. Whoever takes it on will have to figure out what that means, i.e. what is the scope within which the ordering applies... assembly, namespace, etc.

@jnm2
Copy link
Contributor

jnm2 commented Jun 26, 2017

Meaning, do we know that Tom is working on this? It seems that's what you were saying. (Also, bit different, but we do have other issues where the person doing the work isn't assigned because we didn't want to add them as collaborators yet and only collaborators can be assigned. I'd be open to inviting and assigning sooner. Perhaps we should discuss that in a new issue.)

@CharliePoole
Copy link
Contributor Author

@jnm2 Nope, Tom never responded by posting.

FWIW, my approach was to hand out Contributor status right and left to whomever wanted it. That allows assigning things and gives early recognition. By the time they have done a few issues, I'm ready to make them committers for the project to which they contributed.

As I see it, the project lead gets to decide who gets assigned rights and when. @rprouse has indicated that he prefers to wait till after a few issues are done. Having done this for a long time myself, I am very aware of the reasons he may prefer that approach. For one thing, my approach means you
end up with a bunch of non-contributing "contributors." Rob also has not said if others of us should invite contributors, so I don't do that any more. In theory, if allowed by GitHub, you could invite them yourself or you could suggest the names to the project lead.

Anyway, I suggest raising the question with Rob.

@jnm2
Copy link
Contributor

jnm2 commented Jun 26, 2017

Last thing- I know you said this is an easyfix but I tried to get my head around it and to me it still looks like a hardfix 😄 Perhaps a point on how you think it would be implemented would make it seem more doable?

@CharliePoole
Copy link
Contributor Author

It only applies to the relative ordering of tests within the same container - namespace in this case - and so on... That's why it's an easy fix.

Are you rejecting my premise or my conclusion? 😜

@CharliePoole
Copy link
Contributor Author

Also from above comments...

What's needed to implement:

  • Allow the Order attribute on classes
  • Nothing else as far as I can see!

The code at https://github.com/nunit/nunit/blob/master/src/NUnitFramework/framework/Internal/Execution/CompositeWorkItem.cs#L248 that creates work items already looks at fixtures, so it should take care of things automatically if it finds the property has been set.

@jnm2
Copy link
Contributor

jnm2 commented Jun 26, 2017

Are you rejecting my premise or my conclusion?

I wasn't comprehending the premise which tempted me to doubt the conclusion of easiness 😆 You're using terms "relative ordering in a container" I'm not confident with, after the differing expectations on what ordering even means in NUnit.

But I think I understand now that changing the AttributeUsage on the OrderAttribute is all the work that should have to be done. I was getting sidetracked in the other conversations in this thread.

It wouldn't hurt to repeat and summarize the expected work in basic terms when we (I in this case) add the up-for-grabs label, especially in a long thread like this.

@jadarnel27
Copy link
Contributor

Hi! I would like to implement this, as it appears to still be available, and it seems like a fairly simple way to "get my feet wet" in the project.

@ChrisMaddock
Copy link
Member

@jadarnel27 - go for it! I hope it works out as simple as people have suggested!

@rprouse - would you invite @jadarnel27 to the contributors team?

@rprouse
Copy link
Member

rprouse commented Aug 7, 2017

@jadarnel27 I have invited you to the Contributors team. Once you accept, we will be able to assign this issue to you, but feel free to start in the meantime, it is all yours 😄

@jadarnel27
Copy link
Contributor

Thanks, guys! Expect a pull request in the near future 😄

@mikkelbu
Copy link
Member

@jadarnel27 Thanks for the contribution 👍

@jadarnel27
Copy link
Contributor

Glad I could do it! Thanks to all of you for making me feel welcome.

@katyabilokur
Copy link

Hi.
I found this issue discussion and it seems the one I need so much.
I have Telerik Testing Framework + Selenium tests in different classes. One class is one separate test. I need to be able to run my tests in a specific order.

This issue has been discussed for a long time but I couldn't find any solution to it.
Did anyone implement it already by any chance?
From the previous answers, it seems like the fix is pretty simple. Could you tell me exactly what should be changed in that line of code? I will build it for myself then.

Thank you.

@Sebazzz
Copy link

Sebazzz commented Nov 23, 2017 via email

@CharliePoole
Copy link
Contributor Author

This issue was closed as done but it's unclear just what was done. Probably only the easy part of the solution.

Can somebody link the PR to this issue?

@mikkelbu
Copy link
Member

The PR is #2365 (it is also linked to this issue, see just below this comment #345 (comment))

And as far as I can recall it was just the implementation of the following

What's needed to implement:

  • Allow the Order attribute on classes
  • Nothing else as far as I can see!

taken from #345 (comment).

@CharliePoole
Copy link
Contributor Author

@kotiku Looking at the PR, I'm pretty sure that this is fixed.

Note that we made this issue be about simple ordering and not more complex dependencies.

@jadarnel27
Copy link
Contributor

@kotiku just to add to what Charlie said, this was introduced in version 3.8 of NUnit, so make sure you are on that version or later in order to order test fixtures (you could only order individual tests prior to 3.8).

@katyabilokur
Copy link

katyabilokur commented Nov 27, 2017

Hi everyone. Thank you for provided information and references.
Order class attribute does what I need. I have to keep my tests under one namespace to have Order applied but can overcome this. Great job, thanks a lot!

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

Successfully merging a pull request may close this issue.

10 participants