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

Handling failing tests idea #10

Open
Stranger6667 opened this issue Nov 1, 2020 · 3 comments
Open

Handling failing tests idea #10

Stranger6667 opened this issue Nov 1, 2020 · 3 comments

Comments

@Stranger6667
Copy link
Collaborator

Stranger6667 commented Nov 1, 2020

I was thinking about the API for tests and probably it would be nice to keep the test function as close to regular tests as possible. For example:

fn example_test(tc: &mut TestCase) {
    let ls = tc
        .any(&data::Vectors::new(data::Integers::new(95, 105), 9, 11))
        .unwrap();
    assert!(ls.iter().sum::<i64>() <= 1000)
}

Which follows the original minithesis approach:

@run_test(database={}, random=Random(seed))
def _(test_case):
    ls = test_case.any(lists(integers(0, 10000)))
    assert sum(ls) <= 1000

What do you think if we can implement it the same way? For example with std::panic::catch_unwind by handling the Result::Err case. It might be easier for the end-users to write such tests with minithesis-rust.

However, it is more an idea and I am not completely sure if it could be implemented this way

@Rik-de-Kort
Copy link
Owner

Rik-de-Kort commented Nov 1, 2020

I think at the very least we should provide support for these types of tests. Testing is a bit annoying right now to be honest, always having to unwrap and carefully handle the error type.

However, I think we do need to carefully consider the backend implementation: minithesis relies heavily upon the exception mechanism of Python, and I think that's something you don't want to see in Rust. I've made sure all exceptions are proper Err values, but I'm sure the architecture could be improved.

So yes, but don't use this type of exception catching tactic for the implementation.

@Rik-de-Kort
Copy link
Owner

Alright time to do some writing-as-thought here. Apologies if nothing in this comment makes sense, then I didn't finish my line of thought.

The internal architecture of Minithesis is basically as follows.
There are three main objects TestCase, Possibility, and TestingState.
The responsibility of TestCase is to represent a path of choices taken. In essence it's a buffer of choice with some extra structure tacked on. A TestCase can be initialized randomly (for search), or for a specific set of choices.
The responsibility of Possibility is to turn a TestCase instance into an object to use in testing. In Minithesis this is done via the any method on TestCase because debugging needs to be included, but without this info it can live squarely within Possibility. Producing a value might fail because the TestCase doesn't have enough "choice room" (in which case it will yield MTErr::StopTest and set status to MTStatus::Overrun), or because because the test is already "locked" (which might be a synonym for "invalid", but I should think about it. The question here is if your function shouldn't simply already deal with it?).
TestingState is responsible for generating, targeting, and shrinking of examples. It is the primary entry point for actually doing tests, handling test_case creation and randomness internally.

What irks me about the current implementation most is that we are first setting some status code and then returning an error. We use this error in Python to get control back up the call stack, but in the Rust case this is not how we do things. Rather, we use ? to propagate the error upwards. So then my question is, why not simply put the status in the error, forcing the calling code to deal with it. This will be a lot more straightforward than matching for Err(_) and then checking the status.

I'll try having a go at that tomorrow, maybe.

@Rik-de-Kort
Copy link
Owner

Okay that's mostly done seems like, just hunting bugs at this point.

Think we're already good to go for the proc macro!

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