-
Notifications
You must be signed in to change notification settings - Fork 425
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.
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
chore: move playback tests to a separate ci run #1028
Conversation
strategy: | ||
# TODO: test IE 11, Legacy Edge, and New Edge on windows-latest | ||
# test Safari on macos-latest | ||
matrix: | ||
os: [ubuntu-latest] | ||
|
||
test-type: [playback, unit] | ||
env: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved these below matrix, as matrix defines them and it was a bit confusing.
env: | ||
BROWSER_STACK_USERNAME: ${{ secrets.BROWSER_STACK_USERNAME }} | ||
BROWSER_STACK_ACCESS_KEY: ${{ secrets.BROWSER_STACK_ACCESS_KEY }} | ||
CI_TEST_TYPE: ${{matrix.test-type}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ad CI_TEST_TYPE as an environment variable.
scripts/rollup.config.js
Outdated
|
||
if (process.env.CI_TEST_TYPE) { | ||
if (process.env.CI_TEST_TYPE === 'unit') { | ||
options.testInput = ['!test/playback.test.js', 'test/**/*.test.js']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if running unit tests build everything except for playback tests. Otherwise only run playback tests.
2eed290
to
dea9843
Compare
e34d2c4
to
6c70ad8
Compare
@@ -8,81 +8,74 @@ jobs: | |||
runs-on: ubuntu-latest | |||
# Map a step output to a job output | |||
outputs: | |||
should-skip-job: ${{ steps.skip-check.outputs.should_skip }} | |||
should-skip-job: ${{steps.skip-check.outputs.should_skip}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the spacing throughout this file, some had extra spaces, and others did not.
|
||
- name: Read .nvmrc | ||
- name: read node version from .nvmrc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a step name
run: npm test | ||
if: ${{ startsWith(matrix.os, 'ubuntu') }} | ||
|
||
run: npm run test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we test on other operating systems we can worry about all the other code.
"shelljs": "^0.8.2", | ||
"sinon": "^8.1.1", | ||
"url-toolkit": "^2.1.3", | ||
"videojs-contrib-eme": "^3.8.0", | ||
"videojs-contrib-quality-levels": "^2.0.4", | ||
"videojs-generate-karma-config": "~7.0.0", | ||
"videojs-generate-rollup-config": "~5.0.1", | ||
"videojs-generate-rollup-config": "~6.0.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
had to update to the newest videojs-generate-rollup-config so that the multi-entry
plugin supports include/exclude.
@@ -52,8 +52,7 @@ const options = { | |||
worker: worker(), | |||
uglify: terser({ | |||
output: {comments: 'some'}, | |||
compress: {passes: 2}, | |||
include: [/^.+\.min\.js$/] | |||
compress: {passes: 2} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include is no longer an option for this plugin.
Description
Separate playback and unit tests runs in ci so that we can determine if something is breaking or not.