FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

CI: run unit tests on multiple Nodejs versions by kravets-levko · Pull Request #220 · databricks/databricks-sql-nodejs · GitHub

/ databricks-sql-nodejs Public

CI: run unit tests on multiple Nodejs versions #220

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

Merged
kravets-levko merged 3 commits into from
Jan 24, 2024
Conversation 8 Commits 3 Checks 0 Files changed
Merged

CI: run unit tests on multiple Nodejs versions #220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb59c2b
CI: run unit tests on multiple Nodejs versions
kravets-levko Jan 22, 2024
0478a7d
Temporarily exclude Nodejs@14
kravets-levko Jan 22, 2024
1862ea6
Collect reports for all Node versions separately
kravets-levko Jan 23, 2024
File filter

Filter by extension

Filter by extension .json  (1) .yml  (1) All 2 file types selected
Only manifest files
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
49 changes: 30 additions & 19 deletions .github/workflows/main.yml
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -32,30 +32,39 @@ jobs:

unit-test:

benc-db Jan 23, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I recommend separating the unit-tests/linting from the E2E tests. Since the unit tests do not depend on secrets, you can have a one-click approval to run them on public PRs if you separate them into their own Github Action file. That way you can see if a public PR at least passes unit tests before going through the process of making a branch in the local fork in order to run the E2E tests. Another reason to separate them out is that, because they are much cheaper to run, you can have different trigger conditions/exclusions.

kravets-levko Jan 23, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Currently unit and e2e tests are running as a separate jobs, but in single workflow. It should stay this way in order to properly collect coverage data (each job emits own coverage report; separate job waits for test jobs, then retrieves all coverage files and sends them to codecov)

kravets-levko Jan 23, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

+ unit tests indirectly depend on secrets (coverage job needs a Codecov token stored as secret)
runs-on: ubuntu-latest
strategy:
matrix:
# only LTS versions starting from the lowest we support
# TODO: Include Nodejs@14
node-version: ['16', '18', '20']
env:
cache-name: cache-node-modules
NYC_REPORT_DIR: coverage_unit_node${{ matrix.node-version }}

steps:
- uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
- uses: actions/checkout@v3
- name: Cache node modules
uses: actions/cache@v3
with:
path: ~/.npm
key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
key: ${{ runner.os }}-${{ matrix.node-version }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
restore-keys: |
${{ runner.os }}-build-${{ env.cache-name }}-
${{ runner.os }}-build-
${{ runner.os }}-
${{ runner.os }}-${{ matrix.node-version }}-build-${{ env.cache-name }}-
${{ runner.os }}-${{ matrix.node-version }}-build-
${{ runner.os }}-${{ matrix.node-version }}-
- name: Run unit tests
run: |
npm ci
npm run test
- run: tar -cvf coverage_unit.tar coverage_unit
- run: tar -cvf ${{ env.NYC_REPORT_DIR }}.tar ${{ env.NYC_REPORT_DIR }}
- name: Store coverage report
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
with:
name: coverage_unit
path: coverage_unit.tar
name: ${{ env.NYC_REPORT_DIR }}
path: ${{ env.NYC_REPORT_DIR }}.tar
retention-days: 1

e2e-test:
Expand All @@ -67,6 +76,7 @@ jobs:
E2E_ACCESS_TOKEN: ${{ secrets.DATABRICKS_TOKEN }}
E2E_TABLE_SUFFIX: ${{github.sha}}
cache-name: cache-node-modules
NYC_REPORT_DIR: coverage_e2e

benc-db Jan 23, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

For my knowledge, where / how do these get used? It might be a pattern I want to pull into dbt-databricks.

kravets-levko Jan 23, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I use this env here to have unique filenames for coverage reports from different jobs (e.g. coverage_e2e, coverage_unit_node16, etc.), and because that names are used in many places across the workflow file. Those files are later collected by another job which then sends them to Codecov. We cannot send them one by one, because in this case Codecov won't merge those coverage reports and just use the last submitted one

steps:
- uses: actions/checkout@v3
Expand All @@ -83,12 +93,12 @@ jobs:
run: |
npm ci
NODE_OPTIONS="--max-old-space-size=4096" npm run e2e
- run: tar -cvf coverage_e2e.tar coverage_e2e
- run: tar -cvf ${{ env.NYC_REPORT_DIR }}.tar ${{ env.NYC_REPORT_DIR }}
- name: Store coverage report
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
with:
name: coverage_e2e
path: coverage_e2e.tar
name: ${{ env.NYC_REPORT_DIR }}
path: ${{ env.NYC_REPORT_DIR }}.tar
retention-days: 1

coverage:
Expand All @@ -108,14 +118,15 @@ jobs:
${{ runner.os }}-build-${{ env.cache-name }}-
${{ runner.os }}-build-
${{ runner.os }}-
- uses: actions/download-artifact@v3
with:
name: coverage_unit
- uses: actions/download-artifact@v3
- uses: actions/download-artifact@v4
with:
name: coverage_e2e
- run: tar -xvf coverage_unit.tar && rm coverage_unit.tar
- run: tar -xvf coverage_e2e.tar && rm coverage_e2e.tar
pattern: coverage_*
merge-multiple: true
- name: Unpack coverage reports
run: |
ls -1 coverage_*.tar | xargs -I '{}' -- tar -xvf '{}'
rm coverage_*.tar
- run: ls -la
- name: Coverage
uses: codecov/codecov-action@v3
with:
Expand Down
4 changes: 2 additions & 2 deletions package.json
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
},
"scripts": {
"prepare": "npm run build",
"e2e": "nyc --reporter=lcov --report-dir=coverage_e2e mocha --config tests/e2e/.mocharc.js",
"test": "nyc --reporter=lcov --report-dir=coverage_unit mocha --config tests/unit/.mocharc.js",
"e2e": "nyc --reporter=lcov --report-dir=${NYC_REPORT_DIR:-coverage_e2e} mocha --config tests/e2e/.mocharc.js",
"test": "nyc --reporter=lcov --report-dir=${NYC_REPORT_DIR:-coverage_unit} mocha --config tests/unit/.mocharc.js",
"update-version": "node bin/update-version.js && prettier --write ./lib/version.ts",
"build": "npm run update-version && tsc",
"watch": "tsc -w",
Expand Down
Back | FazBrowse Home | New Git URL