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.

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

test: switch from PhantomJS to HeadlessChrome #4262

Merged
merged 3 commits into from
Oct 3, 2019
Merged

test: switch from PhantomJS to HeadlessChrome #4262

merged 3 commits into from
Oct 3, 2019

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Sep 30, 2019

Rework browser tests to run in Headless Chrome instead of PhantomJS, because the latter is no longer maintained.

This allows us to remove transpilation to ES5 via babelify, which significantly improves speed of our tests.

Fixes #4252

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide

@bajtos bajtos added the ci label Sep 30, 2019
@bajtos bajtos requested review from hacksparrow and a team September 30, 2019 07:18
@bajtos bajtos self-assigned this Sep 30, 2019
@bajtos
Copy link
Member Author

bajtos commented Sep 30, 2019

@tomleah it makes me wonder if we can enable browser tests on ppc64 and s390x platform, now that we are using Headless Chrome instead of PhantomJS. What do you think? (These two platforms were disabled by your commit 0dac936.)

@bajtos
Copy link
Member Author

bajtos commented Sep 30, 2019

The build is failing on cis-jenkins because we don't have Chrome installed in the docker image used to run them. I opened a PR to add Chrome, now I am waiting for @rmg to help me get that change landed and published to production.

@bajtos bajtos force-pushed the fix/ci branch 2 times, most recently from 39ab65a to 9dd2257 Compare September 30, 2019 09:31
@bajtos bajtos mentioned this pull request Sep 30, 2019
Closed
@hacksparrow
Copy link
Member

Much better choice. What about the CI failures?

rmg
rmg reviewed Sep 30, 2019
View reviewed changes
test/karma.conf.js Outdated
config.set({
customLaunchers: {
ChromeDocker: {
base: 'ChromeHeadless',
Copy link
Member

Choose a reason for hiding this comment

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

If you change this to ChromiumHeadless it might "just work" since Chromium is installed.

Copy link
Member Author

Choose a reason for hiding this comment

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

In c772233, I added a check to see if chromium or chromium-browser is in PATH and then use ChromiumHeadless instead of ChromeHeadless. Unfortunately, it seems the check did not pass, because the build is still using Chrome (not Chromium):

https://cis-jenkins.swg-devops.com/job/nb/job/loopback%7Emaster/label=x64%20&&%20linux%20&&%20nvm,nodeVersion=10/300/console

Copy link
Member Author

Choose a reason for hiding this comment

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

Below is the Dockerfile I am using to test my setup, it's based on our build images.

###
### Copied from Jenkins base Dockerfile
###

FROM ubuntu:xenial

# Declared first so that during build, nothing ever gets accidentally persisted
VOLUME /tmp

# Simple wrapper for apt-get install that does an update before install and
# cleanup after install to ensure that the resulting layer doesn't contain
# anything extra
COPY docker-friendly-apt-install.sh /bin/docker-friendly-apt-install

# Ensure that package don't give us any trouble when we install them
RUN echo 'DPkg::options { "--force-confdef"; "--force-confnew"; }' \
    > /etc/apt/apt.conf.d/60local

# Use bash by default instead of dash
RUN echo "dash dash/sh boolean false" | debconf-set-selections \
 && DEBIAN_FRONTEND=noninteractive dpkg-reconfigure dash

# Set our locale to use UTF-8 by default
RUN docker-friendly-apt-install locales \
 && locale-gen en_US.UTF-8 \
 && update-locale LANG=en_US.UTF-8 LANGUAGE=en_US:en LC_ALL=en_US.UTF-8
ENV LANG='en_US.UTF-8' LANGUAGE='en_US:en' LC_ALL='en_US.UTF-8'

# ssh and some additional packages that are sufficiently common/popular that
# they are probably wanted/needed in all images.
RUN docker-friendly-apt-install \
      ca-certificates \
      apt-transport-https \
      wget \
      curl \
      git \
      openssh-server \
      zip \
      unzip \
      bzip2 \
      xz-utils \
      bash \
      rsync \
      jq

###
### Install Chromium - see xvfb/Dockerfile
###

# For running protractor/selenium based tests
# libgconf-2-4 is a dependency of the selenium chromedriver
RUN docker-friendly-apt-install xvfb chromium-chromedriver libgconf-2-4

###
### Setup Node environment
###

# Install nvm
RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.34.0/install.sh | bash

RUN source ~/.nvm/nvm.sh && nvm install 12

###
### Run our tests
###

ENV CI=true
ENV JENKINS_HOME=/tmp

COPY "." "."

CMD ["/bin/bash", "-c", "source ~/.nvm/nvm.sh && nvm run 12 ./node_modules/.bin/grunt karma:unit-once"]

I am running the following command to build & run the tests:

docker build -t loopback-core-tests . && docker run -it loopback-core-tests

When I run this on my machine, Chromium is correctly picked and all tests pass.

@rmg could you please help me to investigate why my tests are failing on cis-jenkins?

@tomleah
Copy link
Contributor

tomleah commented Sep 30, 2019

@tomleah it makes me wonder if we can enable browser tests on ppc64 and s390x platform, now that we are using Headless Chrome instead of PhantomJS. What do you think? (These two platforms were disabled by your commit 0dac936.)

As long as Headless Chrome works on those platforms I don't see why not @bajtos. I no longer have access to ppc64 or s390x machines so I'm unable to check this however

@bajtos bajtos force-pushed the fix/ci branch 3 times, most recently from c772233 to a182075 Compare October 1, 2019 07:10
@bajtos
test: switch from PhantomJS to HeadlessChrome
cceb8a3 
Rework browser tests to run in Headless Chrome instead of PhantomJS,
because the latter is no longer maintained.

This allows us to remove transpilation to ES5 via babelify, which
significantly improves speed of our tests.

Signed-off-by: Miroslav Bajtoš <[email protected]>
@rmg
Copy link
Member

rmg commented Oct 2, 2019

@slnode test please

@rmg
Copy link
Member

rmg commented Oct 2, 2019

I made an adjustment to the Jenkins config - it was previously using a minimal image that lacked chromium-browser instead of a more standard image that has it.

@bajtos
Copy link
Member Author

bajtos commented Oct 3, 2019

I made an adjustment to the Jenkins config - it was previously using a minimal image that lacked chromium-browser instead of a more standard image that has it.

Thank you a lot, @rmg! 🙇

@bajtos bajtos requested a review from emonddr October 3, 2019 06:03
@bajtos bajtos mentioned this pull request Oct 3, 2019
Merged
@bajtos bajtos merged commit af9f776 into master Oct 3, 2019
@bajtos bajtos deleted the fix/ci branch October 3, 2019 13:20
@bajtos bajtos mentioned this pull request Oct 7, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmg rmg rmg left review comments

@emonddr emonddr emonddr approved these changes

@hacksparrow hacksparrow Awaiting requested review from hacksparrow

@clark0x clark0x Awaiting requested review from clark0x

@ebarault ebarault Awaiting requested review from ebarault ebarault is a code owner

@fabien fabien Awaiting requested review from fabien fabien is a code owner

@zbarbuto zbarbuto Awaiting requested review from zbarbuto zbarbuto is a code owner

Assignees

@bajtos bajtos

Labels
ci
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix CI builds (Karma + PhantomJS)
5 participants
@bajtos @hacksparrow @tomleah @rmg @emonddr