feat: Add test(should_fail) attribute for tests that are meant to fail

[Ethan-000]

Description

Problem*

Resolves #1994

Summary*

Documentation

• This PR requires documentation updates when merged.

  • I will submit a noir-lang/docs PR.
  • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[phated]

Looking great! I had a few code style recommendations that will clean up a bit

[Ethan-000]

Bikeshedding some more :D, we could probably have the function be:

    pub fn get_all_test_functions<'a>(
        &'a self,
        interner: &'a NodeInterner,
    ) -> impl Iterator<Item = TestFunction> + 'a {
        self.modules
            .iter()
            .flat_map(|(_, module)| module.value_definitions()
            .filter_map(|id| id.as_function()))
            .filter_map(|func_id| 
                match interner.function_meta(&func_id).attributes {
                Some(Attribute::Test(scope)) => Some(TestFunction::new(func_id, scope)),
                _ => None,
            }
        )
    }

ahh thought it would be better to use just one filter_map? i don't have a preference though do u want to go with 2?

[phated]

I think he's trying to remove the internal mapper, so you can combine the two techniques (a flat_map followed by a filter_map`

   pub fn get_all_test_functions<'a>(
        &'a self,
        interner: &'a NodeInterner,
    ) -> impl Iterator<Item = TestFunction> + 'a {
        self.modules
            .iter()
            .flat_map(|(_, module)| module.value_definitions())
            .filter_map(|id| {
                if let Some(func_id) = id.as_function() {
                    match interner.function_meta(&func_id).attributes {
                        Some(Attribute::Test(scope)) => Some(TestFunction::new(func_id, scope)),
                        _ => None,
                    }
                } else {
                    None
                }
            })
    }

technically this is one less closure than what kev suggested, but I'm guessing the compiler would optimize them away 🤔

[kevaundray]

Just going to run this on a few examples

[kevaundray]

This is something we should do in a separate issue, when a test should fail, but it passes. There is no red line to tell us what passed that should not have passed. This is in comparison to the opposite. To illustrate, run nargo test on these two pieces of code:

#[test]
fn foo(){
    assert(1 == 2);
}

#[test(should_fail)]
fn foo(){
    assert(1 == 1);
}

[kevaundray]

This LGTM -- @phated did you have any other concerns?

[phated]

LGTM. Nice work @Ethan-000 - I think we can follow on with more descriptive error messages (such as what kev mentioned or noting that a test failed because it succeeded but was expected to fail)

[Ethan-000]

Thanks for the review 🙂

[Savio-Sou]

Should this be documented (i.e. meant to be used by Noir devs)?

[Ethan-000]

ahh yes should probably be documented with #2541

[Savio-Sou]

Thanks! Created noir-lang/docs#372 for documenting this issue (while #2541 can be documented with noir-lang/docs#367).