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

Get rid of ctx in ctx.it() #4

Open
mackwic opened this issue Jul 23, 2016 · 13 comments
Open

Get rid of ctx in ctx.it() #4

mackwic opened this issue Jul 23, 2016 · 13 comments

Comments

@mackwic
Copy link
Member

mackwic commented Jul 23, 2016

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 ?)
  • ?
@regexident
Copy link
Collaborator

thread_local!(…) might help us here. 🤔

@mackwic
Copy link
Member Author

mackwic commented Aug 16, 2017

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...

@mackwic
Copy link
Member Author

mackwic commented Aug 16, 2017

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

@regexident
Copy link
Collaborator

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. 😞

@mackwic
Copy link
Member Author

mackwic commented Aug 17, 2017

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

@regexident
Copy link
Collaborator

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".

@regexident
Copy link
Collaborator

regexident commented Oct 24, 2017

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.)

@regexident
Copy link
Collaborator

regexident commented Oct 24, 2017

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.

@regexident
Copy link
Collaborator

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
                });
            });
        });
    });
}

@mackwic
Copy link
Member Author

mackwic commented Oct 28, 2017

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

@regexident
Copy link
Collaborator

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.

@ms-ati
Copy link

ms-ati commented May 18, 2018

Any progress on this?

@regexident
Copy link
Collaborator

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

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

No branches or pull requests

3 participants