jupyter / jupyter_kernel_test

A tool for testing Jupyter kernels
BSD 3-Clause "New" or "Revised" License
62 stars 38 forks source link

Support for an optional custom asserter to be used in code_execute_result evaluation #22

Closed poplav closed 8 years ago

poplav commented 8 years ago

Currently code_execute_result verification is done using strict equality on the output message and the user defined expected result.
I'm proposing introducing an optional third element in the sample dictionary called say custom_asserter which is a function where if supplied will be used to validate the output message and the user defined result, instead of strict equality. When working on apache-toree/pull/41 we would like a way to test that addjar works as expected, but need more flexibility in asserting the output due to the nature of the addjar operation.

For reference we recently introduced jupyter_kernel_test into our build process and our tests currently running can be found here.

takluyver commented 8 years ago

Hi @poplav , thanks for the suggestion.

JKT is intended to supply two things: a set of canned fill-in-the-blanks tests to do some basic checks applicable to any kernel, and a convenient interface to write extra tests that are more specific to your kernel. So rather than supplying a custom checker function to pass in to the existing test, you can just write another test. This gives you total flexibility to do what you want, and avoids unnecessary complexity. The interface is pretty simple: test_execute_result is only 15 lines including infrastructure stuff.

If you do add custom tests, I'd recommend calling them e.g. test_toree_*, so that they don't clash with any test methods that might be added to JKT later.

jankatins commented 8 years ago

I would also ask for some support for testing the result: These here: https://github.com/IRkernel/IRkernel/pull/333#issuecomment-219839409 could e.g. be nice regexes

It would already be nice if there would be some shortcuts to write tests ala specify that the result of some submitted stuff should obey rules like "plain text should contain ..." or "contains an error message" or "error message contains 'text'" ...

takluyver commented 8 years ago

What bits need shortcuts to do that? You can already use standard Python code and testing, e.g.:

assert re.match(r'message.*', output['text/plain']), output['text/plain'])
self.assertIn('message', output['text/plain'])
takluyver commented 8 years ago

I've added an example of a custom test in the README. I'm going to close this issue - we can open new ones for any helper methods that might make writing custom tests easier.

jankatins commented 8 years ago

My things were more that these lines are behind a helper function:

https://github.com/jupyter/jupyter_kernel_test/blob/master/jupyter_kernel_test/__init__.py#L197

                self.flush_channels()

                reply, output_msgs = self.execute_helper(sample['code'])

                self.assertEqual(reply['content']['status'], 'ok')

                self.assertGreaterEqual(len(output_msgs), 1)
                self.assertEqual(output_msgs[0]['msg_type'], 'execute_result')
                self.assertIn('text/plain', output_msgs[0]['content']['data'])
                self.assertEqual(output_msgs[0]['content']['data']['text/plain'],
                                 sample['result'])

->

res = self.execute("codeexample") # flushes, submitts, collects results
self.assert_that(res, is_execute_result()) # does the checks above apart from the last line
self.assert_that(display_data_of(res), contains(["plain/text", "text/html"]))
self.assert_that(text_plain_of(res), contains("expected output"))
self.assert_that(text_html_of(res), contains("expected output"))

[I like assert_that style tests :-)]

takluyver commented 8 years ago

[I like assert_that style tests :-)]

assert_that(theBiscuit.getChocolateChipCount(), equal_to(10), 'chocolate chips')

I don't. So I'm not going to integrate that kind of thing into JKT. But feel free to import hamcrest and do whatever you want with it ;-)

jankatins commented 8 years ago

No problem, the above could also be

res = self.execute("codeexample") # flushes, submitts, collects results
self.assert_is_execute_result(res)) # does the checks above apart from the last line
self.assert_display_data_contains(res, ["plain/text", "text/html"])
self.assert_textplain_contains(res, "expected output")
self.assert_texthtml_contains(res, "expected output")

Out of curiosity (up to now I haven't yet written tests in a pyhamcrest style test suite, Rs test_that is the closest yet): what's so bad about this? I always liked the readability of these tests.

takluyver commented 8 years ago
self.assert_is_execute_result(res))

This is already possible as:

from jupyter_kernel_test import validate_message
validate_message(res, 'execute_result')   # + optionally parent=
self.assert_textplain_contains(res, "expected output")

To my mind, that's less clean than self.assertIn("expected output", res['text/plain']). I don't want to make a bunch of hyper-specific assertion functions just to save tiny amounts of typing.

Out of curiosity ... what's so bad about [assert_that style tests]? I always liked the readability of these tests.

They're 'readable' in a kind of clumsy almost-natural-language way, but programming languages already have standard ways to express concepts like 'equal to' (==) and 'contains' (in in Python), and I don't see a need to reinvent them. But worse, when my brain sees something like assert_that(x, equal_to(10)), I waste time and mental energy thinking about how that works, which is totally irrelevant to what I'm trying to test. If it needs a function call, unittest's assertEqual is much less distracting, despite the awkward Java-style naming.

I like py.test, which lets me write assert statements the obvious way and then parses them to give intelligent failure messages.

jankatins commented 8 years ago

Thanks!