rust-rspec/rspec

Get rid of `ctx` in `ctx.it()`

mackwic opened this issue · 13 comments

When declaring an example, we need to keep a reference to the current context, which holds the current group of examples.

Today, it's done by giving the context by argument and calling function on it. Maybe we can do better ?

Ideas:

  • keeping a context stack in a global variable ? (unsafe ?)
  • ?

thread_local!(…) might help us here. 🤔

I'm glad you are motivated to work on Rspec ! 😀

Yes, a thread_local could work, but t the end, we still have the problem of &mut references. For that we will need a mutex...

I tend to think a lot of macro code reposing on hidden conventions could help a lot. But no clear vision at that time...

Hey, by the way, I will be at the Zurich RustFest. Will you be there ?

I'm glad you are motivated to work on Rspec ! 😀

I stumbled upon my old abandoned PR and figured I should get back on it and not leave it half-way finished. 😉

Yes, a thread_local could work, but t the end, we still have the problem of &mut references. For that we will need a mutex...

Yeah. And a reentrant mutex even, I guess. Given that the .with(|cell| { /* … */ }); calls are gonna be nested.

I tend to think a lot of macro code reposing on hidden conventions could help a lot. But no clear vision at that time...

Yeah, that's what I'm thinking, too.

Hey, by the way, I will be at the Zurich RustFest. Will you be there ?

I won't be unfortunately. 😞

Hmm, maybe we don't need a reentrant mutex considering it's only needed in the run_test block

An alternative approach might be to keep ctx. as is but provide an optional compiler plugin with syntax similar to stainless or shiny which would then get expanded to rspec's more verbose syntax.


I'm thinking of something like this:

extern crate rspec;

use std::io;
use std::sync::{Arc, Mutex};
use std::collections::BTreeSet;

use rspec::prelude::*;

pub fn main() {
    let simple = rspec::formatter::Simple::new(io::stdout());
    let formatter = Arc::new(Mutex::new(simple));
    let configuration = Configuration::default().parallel(false);
    let runner = Runner::new(configuration, vec![formatter]);

    let suite = suite!(
        environment {
            set: BTreeSet<usize> = BTreeSet::new(),
            len_before: usize = 0,
        }

        when "not having added any items" {
            then "it is empty" {
                assert!(set.is_empty())
            }
        }

        when "adding an new item" {
            before_all {
                len_before = set.len();
                set.insert(42);
            }

            then "it is not empty any more" {
                assert!(!set.is_empty());
            }

            then "its len increases by 1" {
                assert_eq!(set.len(), len_before + 1);
            }

            when "adding it again", {
                before_all {
                    len_before = set.len();
                    set.insert(42);
                }

                then "its len remains the same" {
                    assert_eq!(set.len(), len_before);
                }
            }
        }

        when "returning to outer context" {
            then "it is still empty" {
                assert!(set.is_empty());
            }
        }

        then "panic!(…) fails" {
            panic!("Some reason for failure.");
        }
    );
    
    runner.run_or_exit(suite);
}

Which would be expanded to this:

extern crate rspec;

use std::io;
use std::sync::{Arc, Mutex};
use std::collections::BTreeSet;

use rspec::prelude::*;

pub fn main() {
    let simple = rspec::formatter::Simple::new(io::stdout());
    let formatter = Arc::new(Mutex::new(simple));
    let configuration = Configuration::default().parallel(false);
    let runner = Runner::new(configuration, vec![formatter]);

    #[derive(Clone, Debug)]
    struct Environment {
        set: BTreeSet<usize>,
        len_before: usize,
    }

    let environment = Environment {
        set: BTreeSet::new(),
        len_before: 0,
    };

    runner.run_or_exit(rspec::given("a BTreeSet", environment, |ctx| {
        ctx.when("not having added any items", |ctx| {
            ctx.then("it is empty", |env| assert!(env.set.is_empty()));
        });

        ctx.when("adding an new item", |ctx| {
            ctx.before_all(|env| {
                env.len_before = env.set.len();
                env.set.insert(42);
            });

            ctx.then("it is not empty any more", |env| {
                assert!(!env.set.is_empty());
            });

            ctx.then("its len increases by 1", move |env| {
                assert_eq!(env.set.len(), env.len_before + 1);
            });

            ctx.when("adding it again", |ctx| {
                ctx.before_all(|env| {
                    env.len_before = env.set.len();
                    env.set.insert(42);
                });

                ctx.then("its len remains the same", move |env| {
                    assert_eq!(env.set.len(), env.len_before);
                });
            });
        });

        ctx.when("returning to outer context", |ctx| {
            ctx.then("it is still empty", |env| assert!(env.set.is_empty()));
        });

        ctx.then("panic!(…) fails", move |_env| {
            panic!("Some reason for failure.")
        });
    }));
}

Haven't spent much thoughts on the details, so consider this more of a "thinking out loud".

I have a working barebones proof-of-concept for a suite-assembly using thread_local!, which would allow for syntax similar to this:

fn main() {
    scenario!("test", env: Environment, || {
        suite!("suite", env: Environment(42), || {
            context!("context", || {
                example!("example", |env| {
                    env.0 == 42
                });
                example!("example", |env| {
                    env.0 == 42
                });
                context!("context", || {
                    example!("example", |env| {
                        env.0 == 42
                    });
                    example!("example", |env| {
                        env.0 == 42
                    });
                });
            });
        });
    });
}

It is a bit involved and rough around the edges but runs on stable!

(The || of || { … } are not strictly necessary, but I kept them for consistency's sake, for now.)

While it doesn't look like too much of an improvement over the current state it allows for optional arguments, such as tags thanks to the use of macros:

fn main() {
    scenario!("test", tags: Tag, env: Environment, || {
        suite!("suite", env: Environment(42), || {
            context!("context", tags: Tag::BAR, || {
                example!("example", tags: Tag::FOO, |env| {
                    env.0 == 42
                });
                example!("example", tags: Tag::BAR, |env| {
                    env.0 == 42
                });
                context!("context", tags: (Tag::FOO | Tag::BAR), || {
                    example!("example", |env| {
                        env.0 == 42
                    });
                    example!("example", |env| {
                        env.0 == 42
                    });
                });
            });
        });
    });
}

The exact semantic model for tags is still TBD.
The proof-of-concept already supports them however. As such the above snippet is working code.

For reference:

The equivalent snippet (sans tags) in rspec's current state would look like this:

fn main() {
    rspec::run(&rspec::suite("suite", Environment(42), |ctx| {
        ctx.context("context", |ctx| {
            ctx.example("example", |env| {
                env.0 == 42
            });
            ctx.example("example", |env| {
                env.0 == 42
            });
            ctx.context("context", |ctx| {
                ctx.example("example", |env| {
                    env.0 == 42
                });
                ctx.example("example", |env| {
                    env.0 == 42
                });
            });
        });
    });
}

Thank you for taking a stab at it ! This is great !

I suggest you can try something on an experiment on the rust-rspec organization. I will do mine, inspired of yours.

I need to try a bit of code before having a definitive opinion on how to do it (for example I think we don't need the quotes, but without trying I'm not sure it's so useful).

Fortunately I have holidays now so I will ave lots of time to test things. :D

I too am on vacation this week. But for me this means no coding as my brand-new MBP is in week-long repair for a broken keyboard.

Looking forward to what you come up with. :).
My PoC is far from polished implementation-wise.

The main advantage—from my point of view—that it would get us is a uniform way to provide support for optional arguments. Without cluttering the api due to Rust’s lack of overloading.

Any progress on this?

@ms-ati We have an experimental macro-based implementation in the experiments repo.