Skip to content

Extend unit tests for the Output widget #1983

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 19 commits into from
Mar 16, 2018
Merged

Extend unit tests for the Output widget #1983

merged 19 commits into from
Mar 16, 2018

Conversation

pbugnion
Copy link
Member

@pbugnion pbugnion commented Feb 25, 2018 •
edited
Loading

This builds on PR #1934, and the diff currently includes some commits from that PR.

While working on PR #1934, I felt that the test coverage on the Python side of the output widget was somewhat sparse. The difficulty is that a lot of the output widget code relies on IPython.display and IPython methods. I've mocked these out in these tests (using manual monkeypatching, rather than introducing a dependency on a mocking library).

  • Is this useful? I personally think this will help developers like me who are less familiar with the codebase, but happy to discard this if maintainers think it adds too much overhead.
  • Should we write this differently? For instance, we could use a real mocking library rather than my home-grown attempts at one. There isn't (AFAICT) much mocking in unit tests at the moment, so it's probably worth settling on one way of doing things.

pbugnion added 19 commits January 26, 2018 14:06
@pbugnion
Add capture decorator for output widget
5725dae
@pbugnion
Docstring for capture decorator.
65250ab
@pbugnion
Add example of decorator to output widget docstring
3df4975
@pbugnion
Avoid changing wrapped descriptors
3de3942 
By using functools.wraps, we avoid overwriting the __repr__ and __doc__ of the
wrapped callback.
@pbugnion
Remove unused import
6b5dfcd
@pbugnion
Add ability to clear output at each new function call
3316b46
@pbugnion
Add docstring to clear_output and capture decorator
499e750
@pbugnion
Rename arguments for consistency
1baed18
@pbugnion
Consistent indentation
6a6dce8
@pbugnion
Add annotation suggesting changes related to when we drop Python2
6a7f333
@pbugnion
Default clear_output to False
9114ee7 
This is more consistent with the behaviour of the context manager, which does
not clear the output every time it is entered.
@pbugnion
Import full output widget module to allow overriding imports
69c0640 
The tests for the output widget now import the entire widget_output submodule,
rather than just the Output widget itself. This allows monkeypatching the locals
in widget_output for easier testing.
@pbugnion
Test that msg_id gets set during capture
bad6467
@pbugnion
Test for clear_output
5075980
@pbugnion
Add test for capture decorator
74054d4
@pbugnion
Test clearing the output in the capture decorator
f4586d2
@pbugnion
Test with capture decorator set to False
825ef56
@pbugnion
Remove redundant checks
469522f
@pbugnion
Docstrings explaining how the mocks work
103cf20
@jasongrout
Copy link
Member

I think this looks great. Thanks!

@jasongrout jasongrout merged commit 8196606 into jupyter-widgets:master Mar 16, 2018
@jasongrout jasongrout added this to the Minor release milestone Mar 16, 2018
@pbugnion pbugnion deleted the output-widget-tests branch March 16, 2018 05:50
@jasongrout jasongrout modified the milestones: Minor release, 7.2 Mar 19, 2018
@github-actions github-actions bot added the resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 7, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Milestone
7.2
Development

Successfully merging this pull request may close these issues.
2 participants
@pbugnion @jasongrout