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

Feature request: Implement "before all" macro #9

Open
64kramsystem opened this issue Oct 26, 2020 · 5 comments
Open

Feature request: Implement "before all" macro #9

64kramsystem opened this issue Oct 26, 2020 · 5 comments
Assignees

Comments

@64kramsystem
Copy link

In some cases, it's required to share a single instance between tests.

AFAIK, the current macro before is run before each test, so it doesn't work for the cases above.

There is a workaround, which is very ugly though - using lazy_static with phony implementations of Send and Sync, which is... 🤯

My request is therefore to implement the (typical) "before_all" logic, which is executed only once (per context/describe).

@aubaugh
Copy link
Owner

aubaugh commented Oct 27, 2020

I'm not sure I understand the advantage of the before_all block. Could you elaborate on sharing a single instance between tests?

You can currently have a nested context/describe block with the root having a before block, which would be executed before all child tests. I could be misunderstanding, but this is what I imagine with a before_all block:

demonstrate! {
    before_all { let x = 10; }
    describe "module1" {
        test "is ten" { assert!(x == 10) }
    }
    describe "module2" {
        test "is not nine" { assert!(x != 9) }
    }
}

but currently with nested describe blocks you can do the following:

demonstrate! {
    describe "root" {
        before { let x = 10; }
        describe "child1" {
            test "is ten" { assert!(x == 10) }
        }
        describe "child2" {
            test "is not nine" { assert!(x != 9) }
        }
    }
}

@64kramsystem
Copy link
Author

64kramsystem commented Oct 28, 2020

edit: clarified the section with the explanation why it doesn't work.

So!

Regarding the drive for this: I need to test a program based on SDL2; a simplified version of the test suite is:

describe "Sdl2Interface" {
    use super::*;

    before {
        let mut interface = Sdl2Interface::init("test");
    }

    it "should initialize with a black canvas" {
        // use `interface`
    }

    it "should write a pixel" {
        // use `interface`
    }

    // other UTs
}

This won't work - UTs are run in parallel, and there can't be multiple interfaces initialized. With a "before all" style, having one shared instance would allow me for example to put a lock (although, I don't know what the demonstrate internals would structure the code).

Regarding the workaround, it works, thanks 😄. I didn't think of it, and previusly ended up with an ugly solution (lazy_static) 😬.

Design-wise, having to introduce an artificial example group hurts the readability of the test suites (as document) though; in frameworks like RSpec there is value in producing documentation through example descriptions (I follow this principle).

Regardless, I think this feature is a convention in testing frameworks - on top of my head, RSpec (Ruby) and JUnit (Java) had it always or at least early. Even discarding my use case, the JUnit describes it as useful, for example for slow initializations (e.g. connecting to a db).

@aubaugh
Copy link
Owner

aubaugh commented Dec 4, 2020

I do think that this is a feature that should be added to this testing framework after reading about the before all block more.

Before I didn't realized that you were wanting to share the same SDL2 interface between tests which is why I recommended using the current before block. This would create a separate SDL2 interface before each test, rather than having the same one that could be shared between the tests with a before all block.

@aubaugh aubaugh self-assigned this Dec 4, 2020
@64kramsystem
Copy link
Author

I do think that this is a feature that should be added to this testing framework after reading about the before all block more.

Before I didn't realized that you were wanting to share the same SDL2 interface between tests which is why I recommended using the current before block. This would create a separate SDL2 interface before each test, rather than having the same one that could be shared between the tests with a before all block.

I thought about this a little more; knowing Rust a bit better than then, I'm actually very perplexed about a "before all"-style functionality, not just in this crate, but in general.

The major conceptual problem is that other testing frameworks are serial (at least, by default), so they don't need to handle concurrent access - and not even as rigorously as Rust does.

Barring unsafe code (which is of course inappropriate), parallel access to an object imply a Mutex, which in turns, implies loss of ownership (since the Mutex will acquire it), and access to an object that is not the intended one (at best, unit tests will receive a MutexGuard). RWLock is another option, but the issues still hold.

Given the conditions, the feature request may be closed on my side - even with a working before_all, given the inherent intricacies, it may be a feature not working.

I've actually found that serial_test is compatible with demonstrate, so there are alternatives (in the test suite of my project, I've actually replaced the SDL layer with a mock one).

@64kramsystem
Copy link
Author

Thanks for the attention, regardless! 🙂

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

No branches or pull requests

2 participants