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

Migrate clicktests from nightmare to puppeteer #1336

Merged
merged 16 commits into from May 6, 2020
Merged

Migrate clicktests from nightmare to puppeteer #1336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
7a8f262
puppeteer
campersau Apr 27, 2020
8a42ce7
reduce test flakiness
campersau Apr 29, 2020
89dfde8
puppeteer does not require xvfb?!
campersau Apr 29, 2020
c49d60f
reduce test flakiness
campersau Apr 30, 2020
1186a7a
don't query same element twice
campersau May 2, 2020
c67d782
try out 1920 x 1080
campersau May 2, 2020
7acac5f
rename nmclicktest -> clicktest
campersau May 2, 2020
c272248
add clicktest npm script
campersau May 2, 2020
a79b887
move headless to enviroment config
campersau May 5, 2020
f551eb2
reduce test flakiness by waiting for the correct element
campersau May 5, 2020
4d7744e
use different commit name for commit & push with ff
campersau May 5, 2020
a112ae7
log selector when click fails
campersau May 5, 2020
934d152
retry click a second time
campersau May 6, 2020
5cc8c2b
remove old gitignore entries
campersau May 6, 2020
62ffbd6
update puppeteer to 3.0.3
campersau May 6, 2020
a204d14
changelog
campersau May 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 2 additions & 4 deletions .github/workflows/ci.yml
View file
Open in desktop
Expand Up @@ -11,8 +11,6 @@ jobs:
os: [ubuntu-latest, macos-latest, windows-latest]
runs-on: ${{ matrix.os }}
env:
# Xvfb (nightmare on linux)
DISPLAY: ':99.0'
# electron packager (win32 ia32 on macos) https://github.com/electron/electron-packager/pull/449#issuecomment-240508298
WINEDLLOVERRIDES: 'mscoree,mshtml='

Expand All @@ -25,10 +23,10 @@ jobs:
node-version: ${{ matrix.node-version }}

# linux dependencies
- run: Xvfb :99 -screen 0 1024x768x24 > /dev/null 2>&1 &
if: matrix.os == 'ubuntu-latest'
- run: sudo apt update
if: matrix.os == 'ubuntu-latest'
- run: sudo apt install -y libgbm1
campersau marked this conversation as resolved.
Show resolved Hide resolved
if: matrix.os == 'ubuntu-latest'
- run: sudo apt install -y wine64
if: matrix.os == 'ubuntu-latest'
- run: wine --version
Expand Down
4 changes: 0 additions & 4 deletions .gitignore
View file
Open in desktop
Expand Up @@ -24,11 +24,7 @@ report/

/node_modules/

/clicktests/screenshots/
/build
/*.dll
/nw.exe
/nw.pak

/components/**/*.css
/components/**/*.bundle.js
Expand Down
1 change: 0 additions & 1 deletion .npmignore
View file
Open in desktop
Expand Up @@ -32,7 +32,6 @@ public/devStyling.html
public/js/devStyling.js

clicktests/
nmclicktests/

teststabilitytester.js

Expand Down
7 changes: 2 additions & 5 deletions .travis.yml
View file
Open in desktop
Expand Up @@ -26,8 +26,7 @@ jobs:
addons:
apt:
packages:
- xvfb
- libgconf-2-4
- libgbm1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

puppeteer/puppeteer@5c839f5

- wine
homebrew:
update: true
Expand All @@ -36,8 +35,6 @@ addons:
- wine-stable
before_install:
# linux
- if [[ $TRAVIS_OS_NAME = "linux" ]]; then export DISPLAY=':99.0'; fi
- if [[ $TRAVIS_OS_NAME = "linux" ]]; then Xvfb :99 -screen 0 1024x768x24 > /dev/null 2>&1 & fi
- if [[ $TRAVIS_OS_NAME = "linux" ]]; then wine --version; fi
- if [[ $TRAVIS_OS_NAME = "linux" && $GIT_VERSION = "edge" ]]; then sudo add-apt-repository ppa:git-core/ppa -y && sudo apt-get update -q && sudo apt-get install -y git; fi
# osx
Expand All @@ -55,7 +52,7 @@ before_script:
- git config --global user.email "test@testy.com"
- git config --global user.name "Test testy"
- git --version
- DISPLAY= npm run package
- npm run package
after_success:
- npm run travisnpmpublish
before_deploy:
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -5,6 +5,9 @@ We are following the [Keep a Changelog](https://keepachangelog.com/) format.

## [Unreleased](https://github.com/FredrikNoren/ungit/compare/v1.5.7...master)

### Changed
- Migrate clicktests from nightmare to puppeteer [#1336](https://github.com/FredrikNoren/ungit/pull/1336)

## [1.5.7](https://github.com/FredrikNoren/ungit/compare/v1.5.6...v1.5.7)

### Fixed
Expand Down
13 changes: 7 additions & 6 deletions Gruntfile.js
View file
Open in desktop
Expand Up @@ -69,18 +69,18 @@ module.exports = (grunt) => {
options: {
reporter: 'spec',
require: './source/utils/winston.js',
timeout: 15000,
timeout: 35000,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

puppeteer default timeout is 30000ms.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would the default be enough?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I also could change the timeout to 15000 which we used before, but opted for sticking with the puppeteer defaults, that's why I have increased it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this is the mocha timeout.

I thought this is setting the puppeteer timeout to 35s instead of 30s and was wondering whether not setting it would be fine.

bail: true
},
src: 'nmclicktests/spec.*.js'
src: 'clicktests/spec.*.js'
}
},

jshint: {
options: {
undef: true, // check for usage of undefined constiables
indent: 2,
esnext: true,
esversion: 6,
'-W033': true, // ignore Missing semicolon
'-W041': true, // ignore Use '===' to compare with '0'
'-W065': true, // ignore Missing radix parameter
Expand Down Expand Up @@ -124,6 +124,7 @@ module.exports = (grunt) => {
},
mocha: {
options: {
esversion: 8,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

async / await

Copy link
Contributor

Choose a reason for hiding this comment

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

Why does JSHint have esversion: 6?
Does it not support anything higher?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because that is what esnext was before https://jshint.com/docs/options/#esnext.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, but can we set it to esversion: 8 for the test files (and node files)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we will but not as part of this PR.

node: true,
globals: {
'it': true,
Expand All @@ -138,7 +139,7 @@ module.exports = (grunt) => {
},
src: [
'test/**/*.js',
'nmclicktests/**/*.js'
'clicktests/**/*.js'
]
}
},
Expand Down Expand Up @@ -305,7 +306,7 @@ module.exports = (grunt) => {
grunt.registerTask('clickParallel', 'Parallelized click tests.', function() {
const done = this.async();

fs.readdirAsync('./nmclicktests')
fs.readdirAsync('./clicktests')
.then((files) => files.filter((file) => file.startsWith('spec.')))
.then((tests) => {
const genericIndx = tests.indexOf('spec.generic.js');
Expand All @@ -321,7 +322,7 @@ module.exports = (grunt) => {

grunt.log.writeln(cliColor.set(`Clicktest started! \t${file}`, 'blue'));
return new Bluebird((resolve, reject) => {
const child = childProcess.execFile('./node_modules/mocha/bin/mocha', [path.join(__dirname, 'nmclicktests', file), '--timeout=20000', '-b'], { maxBuffer: 10*1024*1024 });
const child = childProcess.execFile('./node_modules/mocha/bin/mocha', [path.join(__dirname, 'clicktests', file), '--timeout=35000', '-b'], { maxBuffer: 10*1024*1024 });
child.stdout.on('data', outStream);
child.stderr.on('data', outStream);
child.on('exit', (code) => {
Expand Down