Fix integration tests #5961
Fix integration tests #5961
Conversation
canavandl
commented
Mar 6, 2017
•
canavandl
commented
- issues: fixes Re-enable Saucelabs tests #5947
- tests added / passed
canavandl
changed the title
| self._check_func_called(mock_standalone_html_page, | ||
| (obj, resources, title), | ||
| {}) | ||
| self._check_func_called(mock_io_open, |
There was a problem hiding this comment.
This part of the test is "not great". It'd be better to have io.open generate a tempfile then check the content after writing to it, but I couldn't get that to work.
| @@ -442,10 +442,10 @@ def _get_save_args(state, filename, resources, title): | |||
| def _save_helper(obj, filename, resources, title, validate): | |||
| remove_after = False | |||
| if isinstance(obj, LayoutDOM): | |||
| doc = obj.document | |||
| if doc is None: | |||
| doc = Document().add_root(obj) | |||
There was a problem hiding this comment.
The problem was here, in that Document.add_root doesn't return a value. So trying the call validate on the None value was raising an exception and killing the integration tests.
|
@canavandl, can you also improve integration tests, so that we get an actual exception and not some internal errors, which give no indication of really happened. Because next time I see those internal errors, I will just do |
|
I think I've figured out a route to do that (bubble the test exceptions up to the user report). I'll re-mark this PR as WIP and try to implement. |
|
@canavandl, OK, thanks. |
…ceptions bubble up normally
| report = outcome.get_result() | ||
| xfail = hasattr(report, 'wasxfail') |
There was a problem hiding this comment.
is this currently no-op? I removed it but could add the logic back.
|
I'm currently unable to locally reproduce the test failures (integration tests related to panning in the x-dimension). ping @bryevdv to see if you can diagnose. |
|
Overall, this causes exceptions to bubble up to the user. The html report generation continues to be the same. New pytest log output for tests failing due to non-screenshot-related exceptions:
New pytest log output for tests failing due to screenshot mismatch (you'll see on of the arrows are commented out, causing the mismatch):
|
This is likely related to PR #5349. I will be investigating, though I'm never sure how to do this with integration tests. I would prefer to have those written in JS (plotting + interactions), so that I could run this directly in a browser and observe results. |
| save(plot) | ||
| selenium.get(output_file_url) | ||
| assert has_no_console_errors(selenium) | ||
|
|
||
| # Pan plot and test for new range value | ||
| pan_plot(selenium, pan_x=200, pan_y=0) | ||
| new_range_start = float(selenium.execute_script("""alert(window.get_x_range_start())""")) | ||
| pan_plot(selenium, pan_x=150, pan_y=0) |
There was a problem hiding this comment.
Why change pan range? Does if fail otherwise? This change doesn't seem related to other changes here, so it does in fact fail, maybe there is something more going on underneath.
There was a problem hiding this comment.
The tests fail without the change. One issue was that the pan end was outside of the plot frame, so that the pan event didn't happen.
I'm not 100% sure why this wasn't a problem earlier.
There was a problem hiding this comment.
I'm ok with this change with this explanation and @canavandl reports on local changes. any objection to merging @mattpap ?
There was a problem hiding this comment.
Sounds reasonable, though it looks like there may be some effect of PR #5349. As long as panning works we are fine, but I will have closer look at what happens at the boundaries.
|
To reiterate, it's now possible to run the non-screenshot integration tests locally and use the --pdb flag as expected (which was helpful in debugging this). It's also possible to generate the screenshot report locally via the SauceLab Connect proxy, setting the --driver=SauceLabs and --html=report.html (or any html filename and location) |
|
LGTM |
