Order of test runs in suite

[pkieltyka]

I noticed the tests within a suite run in alphabetical order... why is that..? seems strange. Why wouldn't it follow the same as standard go test cases which is in the order they have been declared?

[pkieltyka]

I see this is an issue with reflect.TypeOf(mySuiteStruct) and it returning the methods in alphabetical order.. am I the only one with this issue? I find with end-to-end states, having separate functions for the states is helpful conceptually, but its breaking in practice

[bbickerton]

It's nice to explicitly test building block methods first, so tests of methods utilizing those can make certain assumptions about the foundational methods and be better focused. Being able to run the tests in a suite in order of definition, as bare tests would be run, or specify a run order (without having to add garbage to the test names to force alphabetical order) would be great.

[ernesto-jimenez]

@pkieltyka @Tauzero what's the use case to have tests run in certain order?

In my opinion, tests should pass independently of heir order. Tests should keep passing if you reorder the definition or change their names.

@Tauzero for what you suggest, you can simply have two different suites and run them in whatever order you prefer.

[leebenson]

@ernesto-jimenez IMO, test order is VERY important. It's common to build on tests sequentially, especially when previous tests have side-effects.

An example I have right now is home-brew session middleware.

The necessary order is:

1. create new session (TestNewSession)
2. add data (TestAddData)
3. retrieve data (TestRetrieveData)
4. update session to "expired" (TestExpireSession)
5. test auto-deletion of expired sessions (TestDeleteSession)

If that stuff runs out of order, I get a slew of runtime error: invalid memory address or nil pointer dereference errors because the steps no longer make any sense.

Alphabetically, the order becomes:

1. add data (TestAddData)
2. test auto-deletion of expired sessions (TestDeleteSession)
3. update session to "expired" (TestExpireSession)
4. create new session (TestNewSession)
5. retrieve data (TestRetrieveData)

In order words...

• I'm adding data (TestAddData) before there's even a session to add to (TestNewSession)
• I'm trying to delete an expired session (TestDeleteSession), that has not been set to expired (TestExpireSession)
• I'm retrieving data (TestRetrieveData) on a session that has already been deleted (TestDeleteSession)

Of course, I could consolidate this functionality into a single test function, but then what would be the point of a test "suite" at all? Why not just have a single go test block and use the assert library?

I could also name the tests "Test1", "Test2", etc - but that's a book keeping headache and doesn't look great on test reports.

I really think this needs to follow the standard pattern that "go test" follows, which I imagine was designed for exactly this reason.

This is an awesome library otherwise.

[leebenson]

Here's my suggested fix...

Like @pkieltyka said, the problem is inherited from reflect alphabetising method names.

You can get the natural order of method definition by sorting the pointer address in memory.

Here's a quick snippet I whipped up. It seems to be reliable, but I have no idea if Go always addresses linearly.

package main

import (
    "fmt"
    "reflect"
    "sort"
)

type t struct{}

func (a *t) A() string { return "A" }
func (a *t) Z() string { return "Z" }
func (a *t) D() string { return "D" }
func (a *t) B() string { return "B" }

type addr struct {
    Addr   uintptr
    Method string
}

type addrList []addr

func (a addrList) Len() int {
    return len(a)
}

func (a addrList) Less(i, j int) bool {
    return a[i].Addr < a[j].Addr
}
func (a addrList) Swap(i, j int) {
    a[i], a[j] = a[j], a[i]
}

func main() {

    methods := addrList{}
    fooType := reflect.TypeOf(&t{})

    for i := 0; i < fooType.NumMethod(); i++ {
        method := fooType.Method(i)
        methods = append(methods, addr{method.Func.Pointer(), method.Name})
    }

    sort.Sort(methods)
    fmt.Println(methods)
}

[ernesto-jimenez]

@leebenson my personal opinion is that relying on test order is a bad practice, since you are basically relying on side effects for your tests to run properly. If somebody changes the order of the tests or defines a test in the middle of the file they could be breaking unrelated tests, which would make things really confusing to tests.

Having a test suite can serve multiple purposes. A couple of quick examples:

• suite can be used to have setup and teardown methods to guarantee tests are independent
• a suite can be used to call it agains two different dependencies. e.g: have the same test suite run using mysql and postgresql

Regarding go test running tests in order of definition. I'm unaware wether the behaviour is by design and intended to stay the same way or just a side effect of how go test is implemented which could be subject to change in future versions of go. Do you have any indication which one it is?

The same goes with your proposed solution: are sequential addresses specified by the core and something that can be relied upon or might it change depending on the architecture or in future versions?

[leebenson]

@ernesto-jimenez sorry to repeat myself, but for edification of those that don't see the PR comments:

What's the downside?

Alphabetical is an arbitrary order by default. It assumes a 1:1 correlation between name and run order.

If your argument is that definition order shouldn't be linked to run order, then that's equally arbitrary and therefore ordering by method definition makes no material difference to users that don't rely on a particular sequence order.

It's exactly the same thing either way - except my way, you're now satisfying users like myself, the two other guys that opened the issue in #194, and no doubt hundreds of others who DO care about order.

Again - what's the downside?

In order words...

• Alphabetical running = forcing people not to care about order definition or names. No-one wins.
• Definition running = exactly the same result for those that don't care about order, but with an additional win to those who do care.

Either way, it's the same result - we get this side-effect for "free".

I don't think forcing a particular pattern on people... or indeed, straying from the go test design serves anyone any better than sticking to convention.

[leebenson]

The same goes with your proposed solution: are sequential addresses specified by the core and something that can be relied upon or might it change depending on the architecture or in future versions?

I have an open request to go-nuts to answer the question of address safety.

In the meantime, if your sort method is arbitrary and mine is arbitrary, why not accept the PR and satisfy those of us relying on it in the interim?

[ernesto-jimenez]

@leebenson current order is the one returned by reflect, which is undocumented and subject to change in future versions. In other words, we are not setting an arbitrary order in the package.

If we consider the order of tests to be important, changing for current reflect's order to a new order is a breaking change. It would break existing test suites that rely on function names to order tests. Which is a big downside.

There's basically a whole set of questions to be answered with regards to test ordering:

Should we guarantee certain order when running tests?

Right now we do not. If the implementation of reflect.Method changes the order in a future Go version and that breaks a test suite, I would argue that's a defect in the test suite relying on side effects rather than a breaking change.

The fact that go test sorts tests by order of definition might be to guarantee repeatable test runs, since having tests run in random order might make tests indeterministic when a test case has an unintended side effect. Running tests in random order would mean tests with unintended side effects could make go test fail randomly, which would make debugging and finding that side effect harder.

I'm yet to find the documentation that specifies go test order is always going to be based on the definition. Do you have any information about why they use the definition order and what are the guarantees of backwards compatibility regarding that order?

If we want to guarantee an order, what should it be?

There's two options:

• Codifying the current alphabetical order. That would protect the package against reflect changing it's internal undocumented behaviour. It would be inconsistent with go test, but compatible with existing test suites that might rely on ordering. People who want tests to run in order would have to be explicit in with their function naming.
• Sorted based on definition. It would be consistent with go test, but break any packages relying on the current alphabetical order and code around could break tests, which to me is always a big smell.

Even if we agreed to go with your option, your solution using addresses will probably invalid since I wouldn't rely on addresses always being based in order of definition. It would be more appropriate to sort based on parsing the source code of the test file. Also, there would still be an big question: how do order tests in a suite which uses composition?

[pkieltyka]

I certainly would like to see tests ordered in the suite, even though I know the conventional thinking is to run tests in an unpredictable order. However, from so many Go code bases I've seen that write plain-jane tests with go test, the ordering helps there, and so the side-effect here as a result of reflect to sort alphabetically is unfortunate. Sorting by memory location could actually work, that is clever.

I also don't think it will break other people's tests because they presumably would be depending on a random-order of tests in the suite anyways.

[leebenson]

@ernesto-jimenez you make some good points - if there's any chance core will de-sequence memory alloc in a future release or when targeting another arch, then obviously it's a brittle strategy and not one we can rely on. The only question is whether pointers are guaranteed to increment. If there's no reallocation of old pointer space, then there's no issue.

As far as which is a preferred strategy- IMO definition order trumps alphabetical. Taking my previous example as literal, there's a much greater mental effort to having to think that TestNewSession comes after TestAddData then there is simply to paste the function below it. I'd argue that in the natural flow of code writing, most programmers tend to think sequentially about their data requirements than out of order. It's a lot quicker to reason that a function goes above/below a related one, than it is to figure out whether "P" comes after "S" in the alphabet, for example - or that "R" needs to be inserted between the two.

You're right, of course, that the ideal scenario is to test in isolation, order be damned. However, I think that's a bit idealistic; in practice, tests usually have side-effect or require a sequence. To follow the isolation philosophy in my session class would mean either creating a session, adding data, and handling the tear-down in a single function (thus negating many of the benefits of running a "suite" of tests), creating multiple suites with many of the same set-up/teardown requirements (i.e. poor refactoring) or having each test create a new session and run that in isolation (possibly introducing bugs that occur in weird edge cases, like performing function B and C against a new session, but not A which would otherwise have already been performed in a sequential test case.) None is ideal.

Function order, by contrast, is a lot easier to reason about, and pragmatic. In the real world, code has side effects and running things out of order against it can throw up strange scenarios. Not being in control of that order naturally limits the "shape" of how the code might otherwise be used in production.

As @pkieltyka, there's both schools of thought used in the wild. I do a lot of front-end stuff in JS, and many of the build tools I use (mocha, gulp, grunt, webpack, etc) wouldn't make any sense out of order. It's probable my experience has been tainted/made biased by this approach, so I apologise if this is a lop-sided argument.

Likewise, I've seen the die-hards saying unit tests should be isolated. But then - isn't the point of a lib like testify to abstract convention? Otherwise why don't we all just throw code into one "Test" function and save ourselves the extra keystrokes?

TL,DR; if we can guarantee order by memory address, it's a free side-effect and I'd encourage opting for it. If not, everything I've said is moot, and I apologise for wasting your time :-)

[ernesto-jimenez]

@pkieltyka:

I certainly would like to see tests ordered in the suite, even though I know the conventional thinking is to run tests in an unpredictable order. However, from so many Go code bases I've seen that write plain-jane tests with go test, the ordering helps there, and so the side-effect here as a result of reflect to sort alphabetically is unfortunate.

Have you seen many Go codebases that rely on order of test execution to run tests with side effects? I haven't.

Sorting by memory location could actually work, that is clever.

I do not thing it would. The fact that memory addresses in this case are incremental is probably just a side effect of the implementation and not guaranteed to always be the case and not guaranteed to be the case in every platform.

I also don't think it will break other people's tests because they presumably would be depending on a random-order of tests in the suite anyways.

The order right now is arbitrary, so some people might be relying on tests running based on alphabetical order. However, if reflect changes the ordering and that breaks their test suite, that's would be a bug in their codebase rather than testify.

@leebenson

The only question is whether pointers are guaranteed to increment. If there's no reallocation of old pointer space, then there's no issue.

Even if the pointers were always incremental in every arch right now, as long as it's unspecified behaviour you should never rely on that, since that would be subject to change at any point.

As far as which is a preferred strategy- IMO definition order trumps alphabetical. [...]

My preferred strategy is no guaranteed order at all ;)

You're right, of course, that the ideal scenario is to test in isolation, order be damned. However, I think that's a bit idealistic; in practice, tests usually have side-effect or require a sequence.

I think it's a totally reasonable requirement. My unit tests are all independent and never rely on ordering for side effects. In my opinion, having tests that rely on side effects to be a bad practice and would rather avoid enabling it.

To follow the isolation philosophy in my session class would mean either creating a session, adding data, and handling the tear-down in a single function (thus negating many of the benefits of running a "suite" of tests)

You can use helper functions or setup and teardown functions.

creating multiple suites with many of the same set-up/teardown requirements (i.e. poor refactoring)

It's go, you can use composition to reuse setup and teardown methods. Some examples can be found here: #215

having each test create a new session and run that in isolation (possibly introducing bugs that occur in weird edge cases, like performing function B and C against a new session, but not A which would otherwise have already been performed in a sequential test case.) None is ideal.

If you are worried about the end to end you should have an isolated integration test. You are trying to rely on execution order to create an integration test split into several unit tests with side effects. If you want to do an integration test, do get all that code within a single isolated test rather than relying on side effects.

Function order, by contrast, is a lot easier to reason about, and pragmatic. In the real world, code has side effects and running things out of order against it can throw up strange scenarios. Not being in control of that order naturally limits the "shape" of how the code might otherwise be used in production.

In Go programs, the only shape requirements within a go file are to start with the package definition, then the imports, and then the rest of the code. You can write a function that uses another function or variable in any other pace within the package. go test is the only part where function definition order is used, and it's unspecified behaviour that is subject to change in the future and you should not rely on.

In my opinion, having code that breaks just by moving function definition around is very bad.

Likewise, I've seen the die-hards saying unit tests should be isolated. But then - isn't the point of a lib like testify to abstract convention? Otherwise why don't we all just throw code into one "Test" function and save ourselves the extra keystrokes?

The purpose of this library is to reduce boilerplate and facilitate testing. In my opinion libs should when possible try to discourage bad practices. In my opinion guaranteeing test ordering would be a bad practice.

I think saying unit tests should be isolated is very reasonable. That's why they are called unit tests after all, and doing so it's not that hard.

[leebenson]

I guess I saw a suite as the "unit" being tested, and not the individual Test* functions within it. In that paradigm, testing in order against the data created by SetupTest doesn't seem so unorthodox, at least to my mind.

In any case, you're obviously right - this is an implementation detail and not one specified in the Go language spec, so my address sort method cannot be relied upon in any case and is therefore moot.

Thanks for engaging.

[ernesto-jimenez]

@leebenson that's probably the issue, you were considering a test suite to be an integration test, but it's just a set of unit tests you want to group together so you can define certain setup and teardown processes.

It's very useful to run the same test suite with different configurations or pre-requisites. e.g: imagine an HTTP API where you have two different roles of users, regular users and admins. Admins must be able to do everything regular users can. You might want to do a test suite and run it two times: one authenticating as a regular user and again as an admin.

I'll be closing this issue. But happy to answer any questions or give you any tips you might have :)

[eduardboamba]

TLDR; ordering tests would be a nice to have feature, but for sure not critical. One can organize tests in a suite however desired, but introducing dependency between them is an extra feature and, again, would be nice to have. I guess it is assumed by most that the definition order is respected because that's how we read the file...
Such dependency can follow a certain strategy, as also mentioned above: one could choose alphabetical order, definition order or specific 1:n execution order (TestA requires the prior execution of TestB and TestC).