-
Notifications
You must be signed in to change notification settings - Fork 3
Extended ci #390
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
Extended ci #390
Changes from all commits
630ae22
05efeb9
2694df8
794d48e
2134d83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| name: Run examples | ||
| description: "Runs all examples on a single architecture and node version" | ||
|
|
||
| inputs: | ||
| build-command: | ||
| required: true | ||
| type: string | ||
|
|
||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - name: Build | ||
| run: ${{ inputs.build-command }} | ||
| shell: bash | ||
| # Scylla docker setup copied from https://github.com/scylladb/scylla-rust-driver/blob/4532bdcf46955af57b902be25e0c419d9d6e3f20/.github/workflows/rust.yml | ||
| - name: Setup 3-node Scylla cluster | ||
| run: | | ||
| sudo sh -c "echo 2097152 >> /proc/sys/fs/aio-max-nr" | ||
| docker compose -f .github/docker-compose.yml up -d --wait | ||
| shell: bash | ||
| - name: Install examples dependencies | ||
| run: | | ||
| cd examples | ||
| npm i | ||
| shell: bash | ||
| - name: Run all examples | ||
| run: SCYLLA_URI=172.42.0.2:9042 npm run examples | ||
| shell: bash | ||
| - name: Stop the cluster | ||
| if: ${{ always() }} | ||
| run: docker compose -f .github/docker-compose.yml stop | ||
| shell: bash | ||
| - name: Print the cluster logs | ||
| if: ${{ always() }} | ||
| run: docker compose -f .github/docker-compose.yml logs | ||
| shell: bash | ||
| - name: Remove cluster | ||
| if: ${{ always() }} | ||
| run: docker compose -f .github/docker-compose.yml down | ||
| shell: bash | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| name: Run integration tests | ||
| description: "Runs integration tests on a single architecture and node version" | ||
|
|
||
| inputs: | ||
| build-command: | ||
| required: true | ||
| type: string | ||
|
|
||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - uses: actions/setup-java@v4 | ||
| with: | ||
| distribution: 'temurin' | ||
| java-version: '8' | ||
| - name: Install ccm | ||
| # We need a copy of the repo for the ssl tests. | ||
| # When installing ccm as a package, we do not save the certificates | ||
| # that are present in the ./ssl directory | ||
| shell: bash | ||
| run: | | ||
| git clone https://github.com/scylladb/scylla-ccm.git | ||
| pip install --user --upgrade psutil | ||
| pip install --user ./scylla-ccm | ||
| - name: Build | ||
| run: ${{ inputs.build-command }} | ||
| shell: bash | ||
| - name: Run integration tests on scylla | ||
| env: | ||
| CCM_IS_SCYLLA: "true" | ||
| CCM_PATH: "./scylla-ccm" | ||
| shell: bash | ||
| run: npm run integration |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,6 @@ | ||
| name: Check code quality | ||
| env: | ||
| DEBUG: napi:* | ||
| APP_NAME: scylladb-javascript-driver | ||
| "on": | ||
| push: | ||
| branches: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
|
|
||
| name: Extended CI | ||
| env: | ||
| DEBUG: napi:* | ||
| PEDANTIC: true | ||
|
|
||
| on: | ||
| push: | ||
| # Currently we create tags before triggering a release. | ||
| # This setup ensures that we run this workflow before the release. | ||
| tags: | ||
| - "*" | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| # The goal of this workflow is to check if examples run without any errors | ||
| # This is a version that tests the full matrix of supported node versions and driver architectures | ||
| extended-examples-test: | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| host: | ||
| - ubuntu-latest | ||
| - ubuntu-24.04-arm | ||
| node: [ 20, 22, 24, current ] | ||
| include: | ||
| - host: ubuntu-latest | ||
| target: x86_64-unknown-linux-gnu | ||
| build: npm run build -- --target x86_64-unknown-linux-gnu | ||
| - host: ubuntu-24.04-arm | ||
| target: aarch64-unknown-linux-gnu | ||
| build: npm run build -- --target aarch64-unknown-linux-gnu | ||
| name: Build and run examples - ${{ matrix.target }} - node@${{ matrix.node }} | ||
| runs-on: ${{ matrix.host }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: ./.github/common/setup | ||
| with: | ||
| host: ${{ matrix.host }} | ||
| target: ${{ matrix.target }} | ||
| node-version: ${{ matrix.node }} | ||
| - uses: ./.github/common/example | ||
| with: | ||
| build-command: ${{ matrix.build }} | ||
adespawn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| extended-integration-tests: | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| settings: | ||
| - host: ubuntu-latest | ||
| target: x86_64-unknown-linux-gnu | ||
| build: npm run build -- --target x86_64-unknown-linux-gnu | ||
| node: [ 20, 22, 24, current ] | ||
| name: Build and run integration tests - ${{ matrix.settings.target }} - node@${{ matrix.node }} | ||
| runs-on: ${{ matrix.settings.host }} | ||
| timeout-minutes: 60 | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: ./.github/common/setup | ||
| with: | ||
| host: ${{ matrix.settings.host }} | ||
| target: ${{ matrix.settings.target }} | ||
| node-version: ${{ matrix.node }} | ||
| - uses: ./.github/common/integration | ||
| with: | ||
| build-command: ${{ matrix.settings.build }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,16 +18,84 @@ file to `.npmignore`. All files listed there will **NOT** be added to the npm pa | |
| When you remove a top level file or directory, remember to check (and remove if present) | ||
| if the deleted file / directory was present in `.npmignore` file. | ||
|
|
||
| ## CI | ||
|
|
||
| We split the CI into two parts: | ||
|
|
||
| - regular CI, that we run on each PR | ||
| - extended CI, that we run on each release | ||
|
|
||
| We also have additional CI, that can be run occasionally: | ||
|
|
||
| - benchmarks (can be triggered manually) | ||
| - documentation release (triggered on pushes to main) | ||
|
|
||
| ### Regular CI | ||
|
|
||
| The regular CI consists of the following workflows: | ||
|
|
||
| - Checking code quality (linters for JS and Rust) | ||
| - TypeScript tests | ||
| - Unit tests | ||
| - JSDoc linter | ||
| - Partial examples (see below) | ||
| - Partial integration tests | ||
|
|
||
| ### Extended CI | ||
|
|
||
| The extended CI consists of the following workflow: | ||
|
|
||
| - Full examples (see bellow) | ||
adespawn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - Full integration tests | ||
|
|
||
| ### Matrix | ||
|
|
||
| We run examples on multiple node versions and architectures. | ||
|
|
||
| - ✅ - means we run it both on regular and extended CI | ||
| - 🟠 - means we run it on extended CI only | ||
| - ❌ - means we do not run this configuration | ||
|
|
||
| The motivation for such split is to reduce the execution time for CI that is run on each commit / PR, | ||
| while also ensuring the driver works correctly in most common configurations, when creating a new release. | ||
|
|
||
| #### Examples | ||
|
|
||
| Linux examples are run with ScyllaDB in a docker container. | ||
|
|
||
| | | Linux x64 | Linux arm | MacOS Intel* | MacOS Arm* | | ||
| |-------------- |----------- |----------- |------------- |------------- | | ||
| | Node 20 | ✅ | 🟠 | ❌ (planned) | ❌ (planned) | | ||
| | Node 22 | 🟠 | 🟠 | ❌ (planned) | ❌ (planned) | | ||
| | Node 24 | 🟠 | 🟠 | ❌ (planned) | ❌ (planned) | | ||
| | Node current | ✅ | ✅ | ❌ (planned) | ❌ (planned) | | ||
|
|
||
| *) Disabled due to problems with docker. There are plans to run them with Cassandra, | ||
| launched through CCM. Split between regular and extended CI is not yet decided. | ||
|
|
||
| #### Integration tests | ||
|
|
||
| | | Linux x64 | Linux arm**| MacOS Intel | MacOS Arm | | ||
| |-------------- |----------- |----------- |------------- |----------- | | ||
| | Node 20 | ✅ | ❌ | ❌ (planned) | ❌ | | ||
| | Node 22 | 🟠 | ❌ | ❌ (planned) | ❌ | | ||
| | Node 24 | 🟠 | ❌ | ❌ (planned) | ❌ | | ||
| | Node current | 🟠 | ❌ | ❌ (planned) | ❌ | | ||
|
Comment on lines
+76
to
+83
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💭 Shouldn't we rather test newest Node.js in the main CI to discover problems with new Node.js implementations quickly? Node 20 should be left in the main CI as well; the remaining Node.js implementations should be run in the extended CI.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to run tests on Linux ARM IMO.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm on the edge here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I had some problems with CCM on arm CI, although I didn't document what specific issue that was.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If CI longevity is the problem for the development, then I agree - let's only run integration tests with the oldest supported Node.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's more of a concern amount of tasks the CI takes - they will be done in parallel.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And what's the concern of the amount of tasks about? CI costs?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When testing #393, I've run into situations where despite running only integration tests CI, some of the run (i've run few in parallel) were waiting for other to finish, before starting. I worry, that when we increase the number or running tasks (especially long one), it may slow down the CI significantly in situations when I push changes to multiple PRs
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or somehow else trigger CI multiple times |
||
|
|
||
| **) Disabled due to problems with ccm | ||
|
|
||
| ## Releasing process | ||
|
|
||
| 1. Bump the package version. Remember to update the version in `package-lock.json` in | ||
| main directory, examples and benchmarks | ||
| (see [example commit](https://github.com/scylladb/nodejs-rs-driver/pull/363/changes/41250609737052975129c7514439869324478008) on how to do that). | ||
| 2. Create a release notes on GitHub. The version tag must match version from `package.json` with `v` prefix (for example: `v0.2.0`). | ||
| 2. Create a new tag | ||
wprzytula marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 3. Ensure the extended CI passes. | ||
| 4. Create release notes on GitHub. The version tag must match version from `package.json` with `v` prefix (for example: `v0.2.0`). | ||
| Once you publish release notes, CI action will trigger automatically. This action will build and publish the npm package. | ||
| 3. Once the CI action finishes, check if it succeeded. If it failed, you will have to fix the underlying issue, and re-run the CI action. | ||
| 4. Verify that the new release is visible at [npmjs site](https://www.npmjs.com/package/scylladb-driver-alpha). | ||
| 5. Test the package, by installing it directly from npm. Go to `examples` directory, in `package.json` update the line: | ||
| 5. Once the CI action finishes, check if it succeeded. If it failed, you will have to fix the underlying issue, and re-run the CI action. | ||
| 6. Verify that the new release is visible at [npmjs site](https://www.npmjs.com/package/scylladb-driver-alpha). | ||
| 7. Test the package, by installing it directly from npm. Go to `examples` directory, in `package.json` update the line: | ||
| `"scylladb-driver-alpha": "file:./../"` | ||
| to: | ||
| `"scylladb-driver-alpha": "<just-released-version>"`, | ||
|
|
||
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.
This is missing in Rust Driver's CI, please add it there too.