ISISComputingGroup / lewis-ess

Let's write intricate simulators!
GNU General Public License v3.0
21 stars 19 forks source link

Update Error Handling to Produce Clearer Invalid Regex Errors #302

Closed JackEAllen closed 2 years ago

JackEAllen commented 3 years ago

Invalid Regex does not currently produce a clear, meaningful error message.

This pull request aims to fix this by returning which function the invalid regex is present within and the invalid regex command as part of a runtime error.

Testing Performed Before Making this Pull Request

mattclarke commented 3 years ago

This looks good - would it be possible to add a test for this?

DominicOram commented 2 years ago

@JackEAllen, I know you were struggling to write the tests due to the dependencies of StreamHandler. I had to deal with this in https://github.com/ess-dmsc/lewis/pull/295/files, maybe you can use some of that?

mattclarke commented 2 years ago

Would something like this work?

def test_func_throws_on_construction_if_regex_invalid():
    f = lambda : 0

    with pytest.raises(RuntimeError):
        func = Func(f, "NOT A REGEX")

If this is possible then it can go into a new test file called test_stream_func.py. If you want extra brownie points you could add tests for the other throwing parts of the constructor if not callable(func):, inspect.getcallargs(func, *[None] * self.matcher.arg_count), etc. ;)

JackEAllen commented 2 years ago

Would something like this work?

def test_func_throws_on_construction_if_regex_invalid():
    f = lambda : 0

    with pytest.raises(RuntimeError):
        func = Func(f, "NOT A REGEX")

If this is possible then it can go into a new test file called test_stream_func.py. If you want extra brownie points you could add tests for the other throwing parts of the constructor if not callable(func):, inspect.getcallargs(func, *[None] * self.matcher.arg_count), etc. ;)

Thank you! I always forget that lambda's are a thing as I'm usually so against using them. But they do have their use cases! Works perfectly for me. Will add a few more unit tests to justify placing in a new file.

DominicOram commented 2 years ago

I think lambdas are awesome, why don't you like them?

mattclarke commented 2 years ago

I've pulled the changes into https://github.com/ess-dmsc/lewis/pull/305 so I could tweak the formatting etc.