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

tools/assertions: fail if exception does not match #225

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

tchaikov
Copy link

before this change, we fail the test if repr(e) does not match with the given matching regular expression. despite that the tests pass as expected, this is what we intend to complete with this verification. let's explain this with a Python REPL session:

>>> import re
>>> ex = Exception('foobarhello')
>>> matching = 'foobar'
>>> regex = re.compile(matching)
>>> actual = repr(e)
>>> actual
"Exception('foobarhello')"
>>> regex.match(actual) is None
True
>>> regex.match("c'est la vie") is None
True

repr() returns a canonical string representation of an object. that's why it adds "Exception()" around the error message. and, it fails to match with the the "matching" string. as "Exception('foobarhello')" does not match at the beginning of this string. but we actually expect the unmatch! in other words, two negatives make a positive.

to correct this, in this change:

  • use str() to stringify the exception, so Python does not include its type in the output string
  • do not check if the return match object is None, but check if it is not None. so the test can fail if the message string in exception does not match.
  • include the message string and regular expression in the raised AssertionError.
  • instead of using 're.match()', use 're.search()' for matching the regular expression with the message extracted from exception, as sometimes, we just pass a part of message in hope to match with the returned message, even if the part is not sitting at the beginning of the message, for instance, the full message looks like: ``` Cannot execute this query as it might involve data filtering and thus may have unpredictable performance. If you want to execute this query despite the performance unpredictability, use ALLOW FILTERING' ```` but we just use "use ALLOW FILTERING". so instead of changing all the tests, update the assert helper.
  • do not compile the re. otherwise we only match it for a single time, and then drop the compiled re on the floor.

before this change, we fail the test if repr(e) does *not* match
with the given `matching` regular expression. despite that the
tests pass as expected, this is what we intend to complete with
this verification. let's explain this with a Python REPL session:

```
>>> import re
>>> ex = Exception('foobarhello')
>>> matching = 'foobar'
>>> regex = re.compile(matching)
>>> actual = repr(e)
>>> actual
"Exception('foobarhello')"
>>> regex.match(actual) is None
True
>>> regex.match("c'est la vie") is None
True
```

`repr()` returns a canonical string representation of an object.
that's why it adds "Exception()" around the error message. and, it fails
to match with the the "matching" string. as "Exception('foobarhello')"
does not match at the beginning of this string. but we actually expect
the unmatch! in other words, two negatives make a positive.

to correct this, in this change:

* use str() to stringify the exception, so Python does not include its
  type in the output string
* do not check if the return match object is None, but check if it is
  *not* None. so the test can fail if the message string in exception
  does not match.
* include the message string and regular expression in the raised
  AssertionError.
* instead of using 're.match()', use 're.search()' for matching the
  regular expression with the message extracted from exception, as
  sometimes, we just pass a part of message in hope to match with
  the returned message, even if the part is not sitting at the beginning
  of the message, for instance, the full message looks like:
  ```
  Cannot execute this query as it might involve data filtering and thus
  may have unpredictable performance. If you want to execute this query
  despite the performance unpredictability, use ALLOW FILTERING'
  ````
  but we just use "use ALLOW FILTERING". so instead of changing all
  the tests, update the assert helper.
* do not compile the re. otherwise we only match it for a single
  time, and then drop the compiled re on the floor.

Signed-off-by: Kefu Chai <[email protected]>
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

Successfully merging this pull request may close these issues.

1 participant