Skip to content

[BEAM-7923] Support docker screendiff integration tests #11507

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
aaltay merged 10 commits into from
Apr 28, 2020
Merged

[BEAM-7923] Support docker screendiff integration tests #11507

aaltay merged 10 commits into from
Apr 28, 2020

Conversation

@nika-qubit
Copy link
Contributor

@nika-qubit nika-qubit commented Apr 23, 2020 •
edited
Loading

  1. Added 2 more chrome options so that the interactive notebook screendiff integration tests work within docker.
  2. Updated the golden screenshot.

Details:

  1. By default, Docker runs a container with a /dev/shm shared memory space 64MB. This is typically too small for Chrome and will cause Chrome to crash when rendering large pages. To fix, run the container with docker run --shm-size=1gb to increase the size of /dev/shm. Since Chrome 65, this is no longer necessary. Instead, launch the browser with the --disable-dev-shm-usage flag
  2. Without the chrome options, a test running within docker container will fail with following error:
    TKBKbwCxjwm

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) 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.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status Build Status
Build Status		 Build Status
Build Status
Build Status
Build Status		 Build Status Build Status Build Status
Build Status
Build Status
Python Build Status
Build Status
Build Status
Build Status		 --- Build Status
Build Status
Build Status		 Build Status
Build Status		 --- --- Build Status
XLang --- --- --- Build Status --- --- Build Status

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status
Build Status		 Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

[BEAM-7923] Support docker screendiff integration tests
82d17fa 
1. Added 2 more chrome options so that the tests work within docker.
2. Updated the golden screenshot.

Change-Id: I9973b53d4aefec94898b55ac3a3625b7b6dcc263
@nika-qubit
Copy link
Contributor Author

nika-qubit commented Apr 23, 2020

R: @aaltay

PTAL, thx!

@aaltay
Copy link
Member

aaltay commented Apr 23, 2020

retest this please

@aaltay
Copy link
Member

aaltay commented Apr 24, 2020

retest this please

aaltay
aaltay reviewed Apr 24, 2020
View reviewed changes
sdks/python/setup.py Outdated
'needle>=0.5.0,<1',
'chromedriver-binary>=80,<81',
# use a fixed version of PIL for different python versions
'pillow==7.1.1',
Copy link
Member

@aaltay aaltay Apr 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Contributor Author

@nika-qubit nika-qubit Apr 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python comes with PIL, but different versions of Python has different PIL versions.
To rule out the difference when taking screenshot, we use a fixed version of PIL explicitly for testing.

Copy link
Member

@aaltay aaltay Apr 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to pin it? Pinning usually results in other issues. Maybe less so here because it is a test dependency.

Copy link
Contributor Author

@nika-qubit nika-qubit Apr 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made it any version under a major version >=7,<8.

kevingg and others added 2 commits April 24, 2020 18:38
@aaltay
Copy link
Member

aaltay commented Apr 27, 2020

retest this please

2 similar comments
@aaltay
Copy link
Member

aaltay commented Apr 27, 2020

retest this please

@aaltay
Copy link
Member

aaltay commented Apr 27, 2020

retest this please

@nika-qubit
Copy link
Contributor Author

nika-qubit commented Apr 27, 2020 •
edited
Loading

The error seems to be affecting multiple precommits.
Created https://issues.apache.org/jira/browse/BEAM-9832 to track the issue.

@aaltay
Copy link
Member

aaltay commented Apr 28, 2020

Run Python PreCommit

5 similar comments
@aaltay
Copy link
Member

aaltay commented Apr 28, 2020

Run Python PreCommit

@aaltay
Copy link
Member

aaltay commented Apr 28, 2020

Run Python PreCommit

@aaltay
Copy link
Member

aaltay commented Apr 28, 2020

Run Python PreCommit

@aaltay
Copy link
Member

aaltay commented Apr 28, 2020

Run Python PreCommit

@aaltay
Copy link
Member

aaltay commented Apr 28, 2020

Run Python PreCommit

@aaltay
Copy link
Member

aaltay commented Apr 28, 2020

retest this please

@aaltay aaltay merged commit bfcb17e into apache:master Apr 28, 2020
yirutang pushed a commit to yirutang/beam that referenced this pull request Jul 23, 2020
@yirutang
[BEAM-7923] Support docker screendiff integration tests (apache#11507)
715551b 
[BEAM-7923] Support docker screendiff integration tests (apache#11507)

1. Added 2 more chrome options so that the tests work within docker.
2. Updated the golden screenshot.

Change-Id: I9973b53d4aefec94898b55ac3a3625b7b6dcc263

* Added explicit wait for facets elements to be rendered and updated the golden image.

* fix lint

* Updated to use a fixed version of PIL and updated the golden image

* Enforce a fixed color profile for chrome.

* Updated golden screenshot once Roboto Mono font is installed.

* Added a threshold for fuzzy match.

Change-Id: I32202d7a959c0df8571621a75f9c564f641fc5b9

* Added support of MacOS(Darwin) and Linux screen diff.

Change-Id: I8397eb0b9ada041a78c455d1afba83e5a9b696e4

* Update the golden for Linux platform.

* Allow installing pillow in a range of versions.

Change-Id: Iecd3800a0944501baa8b9cd4931b3afc4aeb41a0

Co-authored-by: Ning Kang <ningk@ningk-macbookpro.roam.corp.google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aaltay aaltay aaltay approved these changes

Assignees

No one assigned

Labels

python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nika-qubit @aaltay