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

Test the Message Spec #1627

Merged
merged 12 commits into from Apr 18, 2012
Merged

Test the Message Spec #1627

merged 12 commits into from Apr 18, 2012

Conversation

minrk
Copy link
Member

@minrk minrk commented Apr 18, 2012

This adds a few preliminary tests for the message spec.

It uses Traitlets to perform validation of keys.

Checks right now are not very strict, as (almost) any key is allowed to be None, as long as it is defined. This is because I simply do not know which keys are allowed to be None, and this is not discussed in the specification. If no keys are allowed to be None, we violate that all over the place.

Parametric tests are used, so every key validation counts as a test (147!).

Message spec doc was found to misrepresent code in a few points, and some changes were made:

  • spec had error keys as exc_name/value, but we are actually using ename/value (docs updated to match code)
  • payloads were inaccurate - list of dicts, rather than single dict, and transformed_output is a payload, not top-level in exec-reply (docs update to match code).
  • in oinfo_request, detail_level was in message spec, but not actually implemented (code updated to match docs).

History messages are not yet tested, but I think I get at least elementary coverage of everything else in the doc.

@fperez
Copy link
Member

fperez commented Apr 18, 2012

Are you getting these tests run by a simple iptest? I think IPython.zmq is still in our exclusions list... If I run it manually via iptest IPython.zmq, I get dropped into an interactive editor a couple of times... It seems that the zmq subpackage has a couple of nasty tests in it and maybe initially we just punted and disabled the lot. As part of this PR, we should fix that so that iptest picks up all of IPython.zmq for testing as well...

@minrk
Copy link
Member Author

minrk commented Apr 18, 2012

No, I'd been running the individual test file manually (iptest IPython.zmq.tests.test_message_spec).

I'll get on fixing IPython.zmq for general testing.

@fperez
Copy link
Member

fperez commented Apr 18, 2012

Awesome. That way this PR will really have the (fantastic) impact of getting all of our zmq stuff into the regular testing workflow.

Otherwise there are dangling sockets on the Context, which cannot terminate.
it opens a GUI editor, which is obviously inappropriate
@minrk
Copy link
Member Author

minrk commented Apr 18, 2012

okay, iptest now runs IPython.zmq (with gtk/matplotlib exclusions, as appropriate, I believe).

@fperez
Copy link
Member

fperez commented Apr 18, 2012

Awesome! All tetsts pass on my box; we may get a few odd failures tomorrow from the buildbots, but if that's the case we'll track them one by one with the info from Shining Pandas. Heads-up to @takluyver in case anything out of the ordinary happens with py3. But let's merge this as-is, you did a terrific job and I'm thrilled to have zmq and messaging test coverage; we can fine-tune things later as needed.

fperez added a commit that referenced this pull request Apr 18, 2012
Test the Message Spec and add our zmq subpackage to the test suite.

It uses Traitlets to perform validation of keys.

Checks right now are not very strict, as (almost) any key is allowed to be None, as long as it is defined.  This is because I simply do not know which keys are allowed to be None, and this is not discussed in the specification.  If no keys are allowed to be None, we violate that all over the place.

Parametric tests are used, so every key validation counts as a test (147!).

Message spec doc was found to misrepresent code in a few points, and some changes were made:

* spec had error keys as `exc_name/value`, but we are actually using `ename/value` (docs updated to match code)
* payloads were inaccurate - list of dicts, rather than single dict, and transformed_output is a payload, not top-level in exec-reply (docs update to match code).
* in oinfo_request, detail_level was in message spec, but not actually implemented (code updated to match docs).

History messages are not yet tested, but I think I get at least elementary coverage of everything else in the doc.
@fperez fperez merged commit 232fa81 into ipython:master Apr 18, 2012
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Test the Message Spec and add our zmq subpackage to the test suite.

It uses Traitlets to perform validation of keys.

Checks right now are not very strict, as (almost) any key is allowed to be None, as long as it is defined.  This is because I simply do not know which keys are allowed to be None, and this is not discussed in the specification.  If no keys are allowed to be None, we violate that all over the place.

Parametric tests are used, so every key validation counts as a test (147!).

Message spec doc was found to misrepresent code in a few points, and some changes were made:

* spec had error keys as `exc_name/value`, but we are actually using `ename/value` (docs updated to match code)
* payloads were inaccurate - list of dicts, rather than single dict, and transformed_output is a payload, not top-level in exec-reply (docs update to match code).
* in oinfo_request, detail_level was in message spec, but not actually implemented (code updated to match docs).

History messages are not yet tested, but I think I get at least elementary coverage of everything else in the doc.
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.

None yet

2 participants