Skip to content
This repository was archived by the owner on Jun 17, 2021. It is now read-only.

Fix Travis's xvfb service#221

Merged
s1na merged 1 commit intomasterfrom
fix/travis-xvfb
Nov 4, 2019
Merged

Fix Travis's xvfb service#221
s1na merged 1 commit intomasterfrom
fix/travis-xvfb

Conversation

@holgerd77
Copy link
Member

Currently CI is failing due to a common Travis xvfb issue along some distribution update.

This PR fixes the CI build analogue to applied solutions like ethereumjs/merkle-patricia-tree#97.

@alcuadrado Sorry, read your comment on using chrome-headless. I have a huge work back log though atm so I decided to stick to the works-for-sure-since-tested solution. We can nevertheless definitely keep this in discussion respectively do a test/first PR on this on occasion.

Also did some short Google search on this and found these Travis docs, chrome-headless solution seemed to me structurally pretty similar to the current Firefox usage at first sight, might be wrong on this though.

Anyhow, would be glad if we can merge on first round this version to get things going again quickly.

@s1na just had a look that the latest release here is also quite some time ago and we didn't release the new file structure yet (see current closed PR list), together with some other new features (EIP-1191 checksum algorithm support). I would do a subsequent PR after this one is merged - eventually together with some dependency updates - preparing a minor v6.2.0 release, would you go along with this (@alcuadrado as well)?

@holgerd77 holgerd77 requested review from alcuadrado and s1na November 2, 2019 08:17
@holgerd77
Copy link
Member Author

In between update: PR fixes the test:node CI runs, test:browser is still broken. Will have a short look.

@coveralls
Copy link

coveralls commented Nov 2, 2019

Coverage Status

Coverage remained the same at 99.336% when pulling c86f35b on fix/travis-xvfb into 3b10850 on master.

@holgerd77
Copy link
Member Author

Process update: ok, since browser tests are failing anyhow (and also the Firefox solution seems to need some special fixes, see this karma-runner/karma-firefox-launcher#93 issue), I decided to directly do/test the ChromeHeadless switch, CI still running.

@holgerd77
Copy link
Member Author

Ok, this is working now, open for review.

.travis.yml Outdated
- g++-4.8
before_install:
- sh -e /etc/init.d/xvfb start
- google-chrome-stable --headless --disable-gpu --remote-debugging-port=9222 http://localhost &
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary? I have a project using Karma and Chrome headless, and I've never seen this before.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've directly taken this from the Travis docs, might be outdated and reference an older Linux version. Wait, will directly try without and re-report here on the results.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, you're right, not needed.

Ready for a re-review.

@s1na
Copy link
Contributor

s1na commented Nov 4, 2019

@s1na just had a look that the latest release here is also quite some time ago and we didn't release the new file structure yet (see current closed PR list), together with some other new features (EIP-1191 checksum algorithm support). I would do a subsequent PR after this one is merged - eventually together with some dependency updates - preparing a minor v6.2.0 release

You're right, it's been some time since the last release here. That sounds good to me!

Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

Looks good!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants