Allow example groups to order tests either randomly OR as they are defined #766

jwfriese · 2018-01-25T01:20:32Z

This PR attempts to add the ordering functionality requested in #759 .

As written, this PR adds a new argument to describe-type and context-type DSL methods -- an order enum value, which can be one of .defined or .random.

.defined will ensure that the examples are run in the order they are defined within the describe or context blocks. This includes respecting nested describe and context blocks in turn. The new test file (ExplicitOrderingTests.swift) shows you exactly how you can expect it to handle nested describes.

.random will randomize the examples within the describe, although it takes care to respect the order values defined by nested describe and context blocks.

Now, there exist some caveats to note here. The most significant of these is the fact that XCode 9 appears to revert to using alphabetical-by-name ordering of tests only when you use the side tray candies to run the tests. In other words, XCTest does not appear to respect the ordering we set as the return value of testInvocations from QuickSpec. I suspect this is related to #744 and #745, but I'm not sure.

I argue that this PR is still worth adding, because it's still giving many users (anyone using Cmd+U or the command line to run their tests) new goodness. Please let me know if you disagree!

Also, I chose to default the Order to .defined because of the principle of least surprise -- that's closest to how things work now.

Technically, we should probably default to .random since having an explicit ordering to tests is not the best practice.

Here's some stuff I still want to do with this PR once I get some feedback of the high-level design:

~~complete documentation of how to use the feature~~ ✅
~~it's probably unnecessary to maintain childGroups and childExamples as separate collections now within the ExampleGroup class~~ True, but not an optimization that I feel is worth it for now.
~~add some context blocks to the test~~✅
super bonus: allow user to define a sorting function (I actually think this should be done separately though)

QuickBot · 2018-01-25T08:58:36Z

	2 Warnings
⚠️	Need to add an unit test if you’re modifying swift source
⚠️	Big PR

Generated by 🚫 Danger

jwfriese · 2018-02-01T01:23:50Z

By the way, this pull request also solves #152 🎉

This may not be the most lightweight API to allow it, but it achieves it nonetheless.

jwfriese · 2018-02-01T18:17:17Z

@modocache or @jeffh - This has been out here for about a week now, and has a couple of positive reactions. I'd love to hear feedback if you have any. Otherwise, I think this is ready to go! 😃

NachoSoto · 2018-02-01T18:55:14Z

Sources/Quick/DSL/DSL.swift

 }

 /**
    Defines an example group. Equivalent to `describe`.
 */
-public func context(_ description: String, flags: FilterFlags = [:], closure: () -> Void) {
-    World.sharedWorld.context(description, flags: flags, closure: closure)
+public func context(_ description: String, order: Order = Order.defined, flags: FilterFlags = [:], closure: () -> Void) {


Nit: I would avoid duplicating the type in all these: order: Order = .defined.

NachoSoto · 2018-02-01T18:55:38Z

Sources/Quick/DSL/World+DSL.swift

@@ -143,7 +143,7 @@ extension World {
        }
        let callsite = Callsite(file: file, line: line)
        let closure = behavior.spec
-        let group = ExampleGroup(description: behavior.name, flags: flags)
+        let group = ExampleGroup(description: behavior.name, order: Order.defined, flags: flags)


Nit: I’d use type inference here: .defined.

jwfriese · 2018-02-01T19:31:48Z

Thanks for the review and suggestions @NachoSoto 👍

Thiryn · 2018-07-30T09:00:33Z

Just a small up to know if this feature is still considered or not.

jwfriese · 2018-07-30T14:13:24Z

I'd like to know too. I have a few pull requests waiting to be merged.

VojtaStavik · 2018-12-03T10:53:24Z

+1 for merging this 👍

ArtemLyksa · 2018-12-19T15:04:52Z

+1 to merge this. Should be really helpful.

houndci-bot · 2018-12-30T18:40:53Z

Sources/Quick/ExampleGroup.swift

+            #else
+                let d: Int = numericCast(arc4random_uniform(numericCast(unshuffledCount)))
+            #endif
+            let i = randomized.index(firstUnshuffled, offsetBy: d)


Identifier Name Violation: Variable name should be between 3 and 40 characters long: 'i' (identifier_name)

houndci-bot · 2018-12-30T18:40:54Z

Sources/Quick/ExampleGroup.swift

+                srandom(UInt32(time(nil)))
+                let d: Int = Int(random() % (unshuffledCount + 1))
+            #else
+                let d: Int = numericCast(arc4random_uniform(numericCast(unshuffledCount)))


Identifier Name Violation: Variable name should be between 3 and 40 characters long: 'd' (identifier_name)

houndci-bot · 2018-12-30T18:40:54Z

Sources/Quick/ExampleGroup.swift

+        for (firstUnshuffled, unshuffledCount) in zip(randomized.indices, stride(from: randomized.count, to: 1, by: -1)) {
+            #if os(Linux)
+                srandom(UInt32(time(nil)))
+                let d: Int = Int(random() % (unshuffledCount + 1))


Identifier Name Violation: Variable name should be between 3 and 40 characters long: 'd' (identifier_name)

houndci-bot · 2018-12-30T18:40:54Z

Sources/Quick/ExampleGroup.swift

+
+    private var randomizedExamples: [Example] {
+        var randomized = Array(groupUnits)
+        for (firstUnshuffled, unshuffledCount) in zip(randomized.indices, stride(from: randomized.count, to: 1, by: -1)) {


Line Length Violation: Line should be 120 characters or less: currently 122 characters (line_length)

jwfriese · 2018-12-30T18:42:30Z

Since it looks like this feature is still in demand, I am in the process of updating this branch since it's a year old and had a couple merge conflicts. I currently have the tests all passing locally on my Mac. It's running through CI now. I'll fix any breakages that might find.

cmillani · 2019-01-21T15:36:19Z

+1 to merge this, would love to be able to use this 😃

jradivojsa · 2019-02-28T14:30:56Z

I also believe this PR would benefit all of us greatly. To not include such a magnificent feature is nothing short of a sin.
pls murg 👍

cbauer10 · 2019-05-22T21:02:31Z

Any movement on this?

ashikahmad · 2019-07-25T05:42:06Z

Badly missing ordering. Looking forward to see this merged 🤩

ValeFCF · 2019-09-19T21:06:57Z

What is the status for this?

jwfriese · 2019-10-08T01:49:39Z

I have updated this PR now, resolving merge conflicts, and ensuring we still have a clean local rake run. I'll address any check failures as necessary. @ikesyo please let me know if there's anything else you need me to address in order to get this code merged.

ZevEisenberg · 2020-04-20T15:25:59Z

+1 from the future. If you're still in the past, maybe stay there for a bit 😅

ZevEisenberg · 2020-04-20T15:28:17Z

Sources/Quick/ExampleGroup.swift

+                srandom(UInt32(time(nil)))
+                let offset: Int = Int(random() % (unshuffledCount + 1))
+            #else
+                let offset: Int = numericCast(arc4random_uniform(numericCast(unshuffledCount)))


Since Swift 4.2, perhaps this could be replaced with the newer randomness API?

jessesquires · 2022-04-13T17:52:22Z

Hey there! 👋🏼

I'm a new maintainer for this project and I'm trying to get the next release out ASAP and also clean up old issues and old PRs. I'm closing all PRs older than 1-2 years, but that doesn't mean these changes are necessarily being rejected. If you are still interested in contributing these changes, please submit a new PR branching from the top of main.

We appreciate your work here and acknowledge the time and effort you put in to contribute! You're awesome. 💯 However, we are all volunteers here with limited capacity working for free. Unfortunately, that means we must close out stale issues and PRs to move the project forward in a reasonable way.

We apologize for the inconvenience and appreciate your understanding! 😎 ✌🏼

younata · 2023-06-30T17:40:16Z

@jwfriese Hey, I'm coming back to this. I'm curious what you think about changing Quick's default behavior so that it always attempts to execute tests in the order they're defined in (as you've noted, XCTest will ignore that attempt whenever it desires). Additionally, we'll give the guidance that developers should enable "Randomize Execution Order" in the xctestplan when they want to randomize execution order.

This is less configurable than what this PR specifies, but I think overall what I just outlined will be better/easier for people to understand.

jwfriese mentioned this pull request Jan 25, 2018

Add script to run Linux tests in a container using Docker #767

Closed

NachoSoto reviewed Feb 1, 2018

View reviewed changes

jwfriese force-pushed the 759-explicit-test-ordering branch from 4eff3ce to ceec9ca Compare December 30, 2018 18:40

houndci-bot reviewed Dec 30, 2018

View reviewed changes

jwfriese mentioned this pull request Dec 30, 2018

Unique identifiers for all test examples #771

Closed

jwfriese added 9 commits October 8, 2019 12:23

Allow example groups to order tests randomly or as they are defined

c05210a

Use different method of random number generation on Linux

2442b94

Fence use of @objc appropriately

b97adc7

Use swap to randomize test examples when not at least Swift 4

2dcbc8a

Add documentation for the test ordering feature

ce24ba5

Add a context to the ordering tests to prove it can be done

2833057

Simplify explicit references to Order enum values

d548a68

Use more specific variable names

680c516

Fix little issue from resolving merge conflicts

0574b3c

jwfriese force-pushed the 759-explicit-test-ordering branch from ee2058f to 0574b3c Compare October 8, 2019 01:48

ZevEisenberg reviewed Apr 20, 2020

View reviewed changes

jonchui mentioned this pull request May 28, 2020

Allow tests to have a specific order #759

Closed

This was linked to issues Oct 8, 2020

Allow tests to have a specific order #759

Closed

Provide option to randomize test order #152

Closed

Base automatically changed from master to main March 2, 2021 13:54

jessesquires closed this Apr 13, 2022

younata mentioned this pull request Jul 3, 2023

Attempt to run tests within a QuickSpec or AsyncSpec in the order they are defined in #1232

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow example groups to order tests either randomly OR as they are defined #766

Allow example groups to order tests either randomly OR as they are defined #766

jwfriese commented Jan 25, 2018 •

edited

Loading

QuickBot commented Jan 25, 2018 •

edited

Loading

jwfriese commented Feb 1, 2018 •

edited

Loading

jwfriese commented Feb 1, 2018

NachoSoto Feb 1, 2018

NachoSoto Feb 1, 2018

jwfriese commented Feb 1, 2018

Thiryn commented Jul 30, 2018

jwfriese commented Jul 30, 2018

VojtaStavik commented Dec 3, 2018

ArtemLyksa commented Dec 19, 2018

houndci-bot Dec 30, 2018

houndci-bot Dec 30, 2018

houndci-bot Dec 30, 2018

houndci-bot Dec 30, 2018

jwfriese commented Dec 30, 2018

cmillani commented Jan 21, 2019

jradivojsa commented Feb 28, 2019

cbauer10 commented May 22, 2019

ashikahmad commented Jul 25, 2019

ValeFCF commented Sep 19, 2019

jwfriese commented Oct 8, 2019

ZevEisenberg commented Apr 20, 2020

ZevEisenberg Apr 20, 2020

jessesquires commented Apr 13, 2022

younata commented Jun 30, 2023

Allow example groups to order tests either randomly OR as they are defined #766

Allow example groups to order tests either randomly OR as they are defined #766

Conversation

jwfriese commented Jan 25, 2018 • edited Loading

QuickBot commented Jan 25, 2018 • edited Loading

jwfriese commented Feb 1, 2018 • edited Loading

jwfriese commented Feb 1, 2018

NachoSoto Feb 1, 2018

Choose a reason for hiding this comment

NachoSoto Feb 1, 2018

Choose a reason for hiding this comment

jwfriese commented Feb 1, 2018

Thiryn commented Jul 30, 2018

jwfriese commented Jul 30, 2018

VojtaStavik commented Dec 3, 2018

ArtemLyksa commented Dec 19, 2018

houndci-bot Dec 30, 2018

Choose a reason for hiding this comment

houndci-bot Dec 30, 2018

Choose a reason for hiding this comment

houndci-bot Dec 30, 2018

Choose a reason for hiding this comment

houndci-bot Dec 30, 2018

Choose a reason for hiding this comment

jwfriese commented Dec 30, 2018

cmillani commented Jan 21, 2019

jradivojsa commented Feb 28, 2019

cbauer10 commented May 22, 2019

ashikahmad commented Jul 25, 2019

ValeFCF commented Sep 19, 2019

jwfriese commented Oct 8, 2019

ZevEisenberg commented Apr 20, 2020

ZevEisenberg Apr 20, 2020

Choose a reason for hiding this comment

jessesquires commented Apr 13, 2022

younata commented Jun 30, 2023

jwfriese commented Jan 25, 2018 •

edited

Loading

QuickBot commented Jan 25, 2018 •

edited

Loading

jwfriese commented Feb 1, 2018 •

edited

Loading