You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/04/07 22:15:48 UTC
[GitHub] [beam] KevinGG opened a new pull request #11338: [BEAM-7923]
Screendiff Integration Tests
KevinGG opened a new pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338
1. Added a screendiff integration test infrastructure: https://docs.google.com/document/d/1tk2P0Cu9hMjbtDWfIuQEFj07q20iqasqLPVtYVpxw3w/edit?usp=sharing
2. Added the first test notebook: init_square_cube.ipynb with golden
screenshot 7a35f487b2a5f3a9b9852a8659eeb4bd.png and its test: init_square_cube_test.py.
3. To add a new test, simply follow the above example. To generate a new
golden, run tests with `nosetests apache_beam/runners/interactive/testing/integration/tests/xxx_test.py --with-save-baseline`.
4. Added new dependency group [interactive_test].
5. The test is not part of the Jenkins pre-commit process yet. To run it
locally, make sure both [interactive] and [interactive_test]
dependencies are installed, and you have chrome installed.
6. To enable such integration tests on Jenkins, chrome driver plugin
should be installed.
7. The BaseTestCase provided in the screen_diff module supports running
tests for all notebooks under a directory, a single test for a golden
or a single test for a notebook.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay removed a comment on issue #11338: [BEAM-7923]
Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay removed a comment on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613015660
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r406335637
##########
File path: sdks/python/setup.py
##########
@@ -222,6 +222,17 @@ def get_version():
'ipython>=5.8.0,<8',
'timeloop>=1.0.2,<2',
]
+
+INTERACTIVE_BEAM_TEST = [
Review comment:
Fair. Please add this information to the wiki. (https://cwiki.apache.org/confluence/display/BEAM/Python+Tips or a new notebook tips page)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613015850
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r406337263
##########
File path: sdks/python/setup.py
##########
@@ -220,8 +220,21 @@ def get_version():
INTERACTIVE_BEAM = [
'facets-overview>=1.0.0,<2',
'ipython>=5.8.0,<8',
+ 'ipykernel>=5.2.0,<6',
Review comment:
Why is this added here and not to the test list?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613017383
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] KevinGG commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
KevinGG commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-612177734
A Jira ticket was created 3 hours ago about the error https://issues.apache.org/jira/browse/BEAM-9736
Global name `from_container_image` is not defined.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613018252
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r405174955
##########
File path: sdks/python/apache_beam/runners/interactive/testing/integration/notebook_executor.py
##########
@@ -0,0 +1,131 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Module to execute jupyter notebooks and gather the output into renderable
+HTML files."""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import os
+import shutil
+
+from apache_beam.runners.interactive.utils import obfuscate
+
+try:
+ import nbformat
+ from nbconvert.preprocessors import ExecutePreprocessor
+ _interactive_integration_ready = True
+except ImportError:
+ _interactive_integration_ready = False
+
+
+class NotebookExecutor(object):
+ """Executor that reads notebooks, executes it and gathers outputs into static
+ HTML pages that can be served."""
+ def __init__(self, path):
+ # type: (str) -> None
+
+ assert _interactive_integration_ready, (
+ '[interactive_test] dependency is not installed.')
+ assert os.path.exists(path), '{} does not exist.'.format(path)
+ self._paths = []
+ if os.path.isdir(path):
+ for root, _, files in os.walk(path):
+ for filename in files:
+ if filename.endswith('.ipynb'):
+ self._paths.append(os.path.join(root, filename))
+ elif path.endswith('.ipynb'):
+ self._paths.append(path)
+ assert len(
+ self._paths) > 0, ('No notebooks to be executed under{}'.format(path))
+ self._dir = os.path.dirname(self._paths[0])
+ self._output_html_dir = os.path.join(self._dir, 'output')
+ self.cleanup()
+ self._output_html_paths = {}
+ self._notebook_path_to_execution_id = {}
+
+ def cleanup(self):
+ """Cleans up the output folder."""
+ _cleanup(self._output_html_dir)
+
+ def execute(self):
Review comment:
Could we delegate to `nbcovert` to produce the html output? We can reduce most of this function that way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r405182773
##########
File path: sdks/python/apache_beam/runners/interactive/testing/integration/screen_diff.py
##########
@@ -0,0 +1,203 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Module to conduct screen diff based notebook integration tests."""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import os
+import threading
+import unittest
+from http.server import SimpleHTTPRequestHandler
+from http.server import ThreadingHTTPServer
+from multiprocessing import Process
+
+from apache_beam.runners.interactive import interactive_environment as ie
+from apache_beam.runners.interactive.testing.integration import notebook_executor
+
+try:
+ import chromedriver_binary # pylint: disable=unused-import
+ from needle.cases import NeedleTestCase
+ from needle.driver import NeedleChrome
+ from selenium.webdriver.chrome.options import Options
+ _interactive_integration_ready = (
+ notebook_executor._interactive_integration_ready)
+except ImportError:
+ _interactive_integration_ready = False
+
+
+class ScreenDiffIntegrationTestEnvironment(object):
+ """A test environment to conduct screen diff integration tests for notebooks.
Review comment:
Could we compare produced html outputs? Do we need to compare that pixels are equivalent, if instead we can compare the html outputs?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r405182034
##########
File path: sdks/python/apache_beam/runners/interactive/testing/integration/screen_diff.py
##########
@@ -0,0 +1,203 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Module to conduct screen diff based notebook integration tests."""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import os
+import threading
+import unittest
+from http.server import SimpleHTTPRequestHandler
+from http.server import ThreadingHTTPServer
+from multiprocessing import Process
+
+from apache_beam.runners.interactive import interactive_environment as ie
+from apache_beam.runners.interactive.testing.integration import notebook_executor
+
+try:
+ import chromedriver_binary # pylint: disable=unused-import
+ from needle.cases import NeedleTestCase
Review comment:
How does this interact with our regular unittest framework? Could one skip, or run specific tests as needed?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] KevinGG commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
KevinGG commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r406340398
##########
File path: sdks/python/setup.py
##########
@@ -222,6 +222,17 @@ def get_version():
'ipython>=5.8.0,<8',
'timeloop>=1.0.2,<2',
]
+
+INTERACTIVE_BEAM_TEST = [
Review comment:
Will update the readme in another PR once this is submitted
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] KevinGG commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
KevinGG commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r406345320
##########
File path: sdks/python/setup.py
##########
@@ -220,8 +220,21 @@ def get_version():
INTERACTIVE_BEAM = [
'facets-overview>=1.0.0,<2',
'ipython>=5.8.0,<8',
+ 'ipykernel>=5.2.0,<6',
Review comment:
It's also part of the Jupyter notebook setup dependency, see the [readme](https://github.com/apache/beam/tree/master/sdks/python/apache_beam/runners/interactive).
We didn't put `jupyterlab` nor `ipykernel` in `[interactive]` because we wanted to keep the dependency minimal sized. Since now we may need `ipykernel` in the virtual env setup, add it to `[interactive]` because it's where it should go.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-612264842
> A Jira ticket was created 3 hours ago about the error https://issues.apache.org/jira/browse/BEAM-9736
>
> Global name `from_container_image` is not defined.
This is marked as fixed now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] KevinGG commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
KevinGG commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r405729704
##########
File path: sdks/python/apache_beam/runners/interactive/testing/integration/notebook_executor.py
##########
@@ -0,0 +1,131 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Module to execute jupyter notebooks and gather the output into renderable
+HTML files."""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import os
+import shutil
+
+from apache_beam.runners.interactive.utils import obfuscate
+
+try:
+ import nbformat
+ from nbconvert.preprocessors import ExecutePreprocessor
+ _interactive_integration_ready = True
+except ImportError:
+ _interactive_integration_ready = False
+
+
+class NotebookExecutor(object):
+ """Executor that reads notebooks, executes it and gathers outputs into static
+ HTML pages that can be served."""
+ def __init__(self, path):
+ # type: (str) -> None
+
+ assert _interactive_integration_ready, (
+ '[interactive_test] dependency is not installed.')
+ assert os.path.exists(path), '{} does not exist.'.format(path)
+ self._paths = []
+ if os.path.isdir(path):
+ for root, _, files in os.walk(path):
+ for filename in files:
+ if filename.endswith('.ipynb'):
+ self._paths.append(os.path.join(root, filename))
+ elif path.endswith('.ipynb'):
+ self._paths.append(path)
+ assert len(
+ self._paths) > 0, ('No notebooks to be executed under{}'.format(path))
+ self._dir = os.path.dirname(self._paths[0])
+ self._output_html_dir = os.path.join(self._dir, 'output')
+ self.cleanup()
+ self._output_html_paths = {}
+ self._notebook_path_to_execution_id = {}
+
+ def cleanup(self):
+ """Cleans up the output folder."""
+ _cleanup(self._output_html_dir)
+
+ def execute(self):
Review comment:
It's tempting to switch to `nbconvert`'s `HTMLExporter`, however, it has several drawbacks.
- The HTMLExporter uses Jinja2 templates. Some of the templates such as `basic` generates broken JavaScripts while some of the templates such as `full` includes too many web elements ;
- There is no guarantee that template provided by `nbconvert` will be stable in future `nbconvert` releases;
- We have to rule out all code blocks that generates non-stable outputs in test notebooks even if they are not related to Interactive Beam and should not be included in the screenshot.
- The more complicated the HTML is, the more flaky the screen diff test will be.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] KevinGG commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
KevinGG commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r405744990
##########
File path: sdks/python/apache_beam/runners/interactive/testing/integration/tests/init_square_cube_test.py
##########
@@ -0,0 +1,37 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Integration tests for interactive beam."""
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import unittest
+
+from apache_beam.runners.interactive.testing.integration.screen_diff import BaseTestCase
+
+
+@unittest.skipIf(
+ BaseTestCase.should_skip(),
+ '[interactive] and [interactive_test] dependency are both required.')
+class InitSquareCubeTest(BaseTestCase):
+ def test_init_square_cube_notebook(self):
+ self.assert_notebook('init_square_cube')
Review comment:
As explained in the
> Could we compare produced html outputs? Do we need to compare that pixels are equivalent, if instead we can compare the html outputs?
The advantages/benefits is that:
- This is real integration test that executes/renders the notebook outputs;
- This can find errors and problems that regex/plain-text comparison could not catch;
- This rules out false test failures (flakiness) in regex/plain-text comparison tests.
- The test result is screenshot that is highly readable.
- You don't even need to write any test. Just give the infrastructure a notebook and you are done.
It does have its own disadvantage/flakiness:
- Anything that causes unstable rendering will potentially fail the test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613016070
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-612264898
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613015562
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay removed a comment on issue #11338: [BEAM-7923]
Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay removed a comment on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613016070
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] KevinGG commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
KevinGG commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r405731238
##########
File path: sdks/python/apache_beam/runners/interactive/testing/integration/screen_diff.py
##########
@@ -0,0 +1,203 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Module to conduct screen diff based notebook integration tests."""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import os
+import threading
+import unittest
+from http.server import SimpleHTTPRequestHandler
+from http.server import ThreadingHTTPServer
+from multiprocessing import Process
+
+from apache_beam.runners.interactive import interactive_environment as ie
+from apache_beam.runners.interactive.testing.integration import notebook_executor
+
+try:
+ import chromedriver_binary # pylint: disable=unused-import
+ from needle.cases import NeedleTestCase
Review comment:
It is a normal unit test.
This will be picked up and skipped (since I don't intend to put it in Beam pre-commit yet) with existing test suites.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay merged pull request #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay merged pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-612166350
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] KevinGG commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
KevinGG commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r406351185
##########
File path: sdks/python/setup.py
##########
@@ -220,8 +220,21 @@ def get_version():
INTERACTIVE_BEAM = [
'facets-overview>=1.0.0,<2',
'ipython>=5.8.0,<8',
+ 'ipykernel>=5.2.0,<6',
Review comment:
It's actually related, because the nbconvert module requires an ipykernel to work.
So this dependency is needed by both `[interactive]` and `[interactive_test]`.
It looks weird to put it in `[interactive]` rather than `[interactive_test]` though :), but that's where it should have been.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613149907
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] KevinGG commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
KevinGG commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r405754337
##########
File path: sdks/python/setup.py
##########
@@ -222,6 +222,17 @@ def get_version():
'ipython>=5.8.0,<8',
'timeloop>=1.0.2,<2',
]
+
+INTERACTIVE_BEAM_TEST = [
Review comment:
TL;DR: It is a normal unit test that works with every platform.
For the time being, I'll not integrate this into Beam pre-commit.
I've marked such tests `skip` conditionally for both `pytest` and `unittest`.
I'll first integrate this into the hosted notebook release process on cloud build.
When the test is stable, we can discuss whether including a chrome executable on Jenkins is worth it.
-- Below assumes we don't want to skip these tests and removed the skip marks --
Tox integration:
As a normal unit test, it automatically gets included by our existing tox setup.
However, to allow the test being executed successfully by tox:
- add [interactive] and [interactive_test] as extras in tox testenvs, e.g., py37
- add an ipython kernel to the runtime environment (programmatically done in notebook_executor.py)
Then you can run
```
# --recreate is needed the first time you make a change to tox.ini.
tox --recreate -e py37 apache_beam/runners/interactive/testing/integration/tests/init_square_cube_test.py
```
You can already run it with `pytest`, `unittest`, `nosetests`:
For example, under `.../beam/sdks/python`,
```
# pytest
pytest -v apache_beam/runners/interactive/testing/integration/tests/init_square_cube_test.py
# unittest
python -m unittest apache_beam/runners/interactive/testing/integration/tests/init_square_cube_test.py
# nosetests
nosetests apache_beam/runners/interactive/testing/integration/tests/init_square_cube_test.py
```
Jenkins integration:
The only dependency that is "hard" to set up is pre-installing a chrome executable on Jenkins.
We'll visit https://github.com/apache/beam/tree/master/.test-infra/dockerized-jenkins in the future.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] KevinGG commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
KevinGG commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-610647985
yapf formatted.
lint passed locally.
R: @aaltay
R: @davidyan74
R: @rohdesamuel
PTAL, thx!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay removed a comment on issue #11338: [BEAM-7923]
Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay removed a comment on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613015562
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r406325631
##########
File path: sdks/python/apache_beam/runners/interactive/testing/integration/notebook_executor.py
##########
@@ -0,0 +1,131 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Module to execute jupyter notebooks and gather the output into renderable
+HTML files."""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import os
+import shutil
+
+from apache_beam.runners.interactive.utils import obfuscate
+
+try:
+ import nbformat
+ from nbconvert.preprocessors import ExecutePreprocessor
+ _interactive_integration_ready = True
+except ImportError:
+ _interactive_integration_ready = False
+
+
+class NotebookExecutor(object):
+ """Executor that reads notebooks, executes it and gathers outputs into static
+ HTML pages that can be served."""
+ def __init__(self, path):
+ # type: (str) -> None
+
+ assert _interactive_integration_ready, (
+ '[interactive_test] dependency is not installed.')
+ assert os.path.exists(path), '{} does not exist.'.format(path)
+ self._paths = []
+ if os.path.isdir(path):
+ for root, _, files in os.walk(path):
+ for filename in files:
+ if filename.endswith('.ipynb'):
+ self._paths.append(os.path.join(root, filename))
+ elif path.endswith('.ipynb'):
+ self._paths.append(path)
+ assert len(
+ self._paths) > 0, ('No notebooks to be executed under{}'.format(path))
+ self._dir = os.path.dirname(self._paths[0])
+ self._output_html_dir = os.path.join(self._dir, 'output')
+ self.cleanup()
+ self._output_html_paths = {}
+ self._notebook_path_to_execution_id = {}
+
+ def cleanup(self):
+ """Cleans up the output folder."""
+ _cleanup(self._output_html_dir)
+
+ def execute(self):
Review comment:
OK this is fair.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r405182991
##########
File path: sdks/python/apache_beam/runners/interactive/testing/integration/tests/init_square_cube_test.py
##########
@@ -0,0 +1,37 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Integration tests for interactive beam."""
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import unittest
+
+from apache_beam.runners.interactive.testing.integration.screen_diff import BaseTestCase
+
+
+@unittest.skipIf(
+ BaseTestCase.should_skip(),
+ '[interactive] and [interactive_test] dependency are both required.')
+class InitSquareCubeTest(BaseTestCase):
+ def test_init_square_cube_notebook(self):
+ self.assert_notebook('init_square_cube')
Review comment:
High level question, do we need to compare that pixels did not change with chnages? What benefit do we get from it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r405183151
##########
File path: sdks/python/setup.py
##########
@@ -222,6 +222,17 @@ def get_version():
'ipython>=5.8.0,<8',
'timeloop>=1.0.2,<2',
]
+
+INTERACTIVE_BEAM_TEST = [
Review comment:
This is a quite a large addition for the value we are getting.
And,
- should this be integrated with tox?
- jenkins?
- which platforms does this test work in?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r406346320
##########
File path: sdks/python/setup.py
##########
@@ -220,8 +220,21 @@ def get_version():
INTERACTIVE_BEAM = [
'facets-overview>=1.0.0,<2',
'ipython>=5.8.0,<8',
+ 'ipykernel>=5.2.0,<6',
Review comment:
Got it. This is fine. I got confused because I could not tell its relation to this PR. Please break different changes to different PRs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on issue #11338: [BEAM-7923] Screendiff
Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613015660
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] KevinGG commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
KevinGG commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r405740866
##########
File path: sdks/python/apache_beam/runners/interactive/testing/integration/screen_diff.py
##########
@@ -0,0 +1,203 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Module to conduct screen diff based notebook integration tests."""
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import os
+import threading
+import unittest
+from http.server import SimpleHTTPRequestHandler
+from http.server import ThreadingHTTPServer
+from multiprocessing import Process
+
+from apache_beam.runners.interactive import interactive_environment as ie
+from apache_beam.runners.interactive.testing.integration import notebook_executor
+
+try:
+ import chromedriver_binary # pylint: disable=unused-import
+ from needle.cases import NeedleTestCase
+ from needle.driver import NeedleChrome
+ from selenium.webdriver.chrome.options import Options
+ _interactive_integration_ready = (
+ notebook_executor._interactive_integration_ready)
+except ImportError:
+ _interactive_integration_ready = False
+
+
+class ScreenDiffIntegrationTestEnvironment(object):
+ """A test environment to conduct screen diff integration tests for notebooks.
Review comment:
I've listed several problems with that approach in https://docs.google.com/document/d/1tk2P0Cu9hMjbtDWfIuQEFj07q20iqasqLPVtYVpxw3w/edit?usp=sharing.
TL;DR: We can and we have such tests in place that compares the produced html outputs, but the screen diff test covers a different test scenario and should not be mutual excluded.
- Many web elements are not stable across multiple runs. So we cannot simply compare the HTML as plain text between runs.
- We have regex assertions in hosted notebook image release integration tests. However, regex is tedious to write, error-prone, not readable and could not cover all outputs.
- There are changes we/users do not care but could cause unexpected test failure. For example, refactoring a piece of Javascript/HTML template that does not affect the final web page rendered should not cause integration test to fail.
- There are changes we do care but will not cause test failure when expected. For example, when modern browser drops support to some old technology that breaks the rendering of web page even when the output HTML has not changed at all, such integration test should catch that.
- Furthermore, asserting generated HTML/JavaScript/CSS is like asserting source code without executing them, thus missing runtime errors. There are situations when a malformed CSS selector could break JavaScripts, asserting notebook outputs as text will not catch it until you run the actual JavaScripts and capture the screenshot.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay removed a comment on issue #11338: [BEAM-7923]
Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay removed a comment on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613015850
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] KevinGG commented on a change in pull request #11338:
[BEAM-7923] Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
KevinGG commented on a change in pull request #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#discussion_r406351185
##########
File path: sdks/python/setup.py
##########
@@ -220,8 +220,21 @@ def get_version():
INTERACTIVE_BEAM = [
'facets-overview>=1.0.0,<2',
'ipython>=5.8.0,<8',
+ 'ipykernel>=5.2.0,<6',
Review comment:
It's actually related, because the nbconvert module requires an ipykernel to work.
So this dependency is needed by both `[interactive]` and `[interactive_test]`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay removed a comment on issue #11338: [BEAM-7923]
Screendiff Integration Tests
Posted by GitBox <gi...@apache.org>.
aaltay removed a comment on issue #11338: [BEAM-7923] Screendiff Integration Tests
URL: https://github.com/apache/beam/pull/11338#issuecomment-613017383
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services