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.

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

Create GitHub Actions Test Setup #652

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

ronilan
Copy link
Contributor

@ronilan ronilan commented May 12, 2022

Overview

This pull request introduces a GitHub Actions test setup that runs tests against both the mock S3 and a real S3 bucket. It also includes a beefed up NW.js test. The Action covers (roughly) all that is covered in the Travis and AppVeyor setups. It can run from any forked repo independent of Mapbox bucket setups (see: #613).

This pull request comes "on top" of (i.e. includes changes from) #651 (which in turn comes "on top" of #648, #649, #650).

Configuration

Running the tests against a real S3 bucket requires configuring the following repo secrets:

  • AWS_ACCESS_KEY_ID
  • AWS_SECRET_ACCESS_KEY
  • S3_BUCKET

When not configured, the S3 tests are skipped.

Change

  • Created GitHub Actions Test Setup (7b8b09a).

    • Added a GitHub Actions workflow that runs whenever there is a push to the repo.
    • Workflow includes two jobs:
      • A matrix job of node versions (10, 12, 14, 16, 18) and operating systems (Linux (ubuntu), Mac and Windows (2019 Enterprise)) that runs all tests against mock and then runs s3 tests against a bucket (located at us-east-1-bucket) specified as a repo secret.
      • A matrix job of and NW.js versions (0.64.0, 0.50.2) and node versions (10, 12, ,14, 16) that runs the NW.js test script.
  • Modified existing configurations to work with GitHub Actions Test setup (4cbbc79).

    • Modified scripts/test-node-webkit.sh so that it can now accept an NW.js version as input. This allows running the script in a GitHub Actions matrix.
    • Modified test/run.util.js so that it does not set --msvs_version=2015 when running in GitHub Actions. This is required because current GitHub Actions runner do not support VS Studio 2015.
    • Added npm script command test:s3 to package.json that runs only the s3 tests. This is required because invoking npx tape test/s3.test.js on windows does not work as expected.
    • Modified test/proxy-bcrypt.test.js. Removed uneeded CI conditionals and modified download directory setup/cleanup. Latter was required due to concurrency issues with running tests on Mac, resulting in uncatchable errors during directory removal originating from rimraf.

Test

  • All tests pass in all configurations (Local Mac, GitHub Actions, Travis, AppVeyor)

lib/mock/http.js Fixed Show fixed Hide fixed
lib/mock/http.js Fixed Show fixed Hide fixed
Copy link
Collaborator

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

Too many changes in a single PR.

It would be great to see a PR that only creates the GitHub Action. Other mods can be put into separate PRs once Appveyor and GitHub Actions are passing on modern versions of Node.js.

.github/workflows/push.yml Outdated Show resolved Hide resolved
.github/workflows/push.yml Outdated Show resolved Hide resolved
.github/workflows/push.yml Outdated Show resolved Hide resolved
@ronilan ronilan force-pushed the Test-Create-GitHub-Actions-Test-Setup branch 6 times, most recently from 5d223a2 to aedb181 Compare June 29, 2024 07:10
@ronilan
Copy link
Contributor Author

ronilan commented Jun 29, 2024

Too many changes in a single PR.

It would be great to see a PR that only creates the GitHub Action. Other mods can be put into separate PRs once Appveyor and GitHub Actions are passing on modern versions of Node.js.

This one comes on top of #651.
When #651 is merged all that is left in ##652 (this one) is the Push GitHub Action.

@ronilan ronilan requested a review from cclauss June 29, 2024 07:13
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest] # due to node-gyp & node compatibility issues, windows 2022 won't work for all node versions
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this comment still needed?

Can we come up with a more self-documenting name than push as other GHAs also run on git push.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we come up with a more self-documenting name than push as other GHAs also run on git push.

Yep.

Given the new ci action, I removed the redundant run of tests and renamed the leftover for what they are (S3 Tests).

Once the bucket is created (note: Object Ownership -> ACLs enabled) and repo secrets are set this works as expected (see: https://github.com/ronilan/node-pre-gyp/actions/runs/9722621262)

@ronilan ronilan requested a review from cclauss June 29, 2024 07:21
ronilan and others added 12 commits June 30, 2024 14:27
- mocking setup can be more easily used by automated tests and/or manually by developer.
- mock modules moved to lib/mock.
- mock modules are self contained and have a signature similar to that of other modules in the package.
- simplified http mock to remove no longer needed interface.
- modified http mock to support path style bucket access.
- set http mock to work with default npg-mock-bucket bucket.
- separated s3 tests from build tests.
- refactored fetch test and proxy-bcrypt test to work with refactored mock setup.
- set hosts of test apps to point to mock bucket.
- added app1.1 - identical to app1 but using production and staging binary host option.
- added app1.2 - identical to app1 but using explicit host, region, bucket options.
…is versa. Removed previous one.

Fix CodeQL errors.
- Added a GitHub Actions workflow that runs whenever there is a push to the repo.
- Workflow includes two jobs:
    - A matrix job of node versions (10, 12, 14, 16, 18) and operating systems (Linux (ubuntu), Mac and Windows (2019 Enterprise)) that runs all tests against mock and then runs s3 tests against a bucket (located at us-east-1-bucket) specified as a repo secret.
    - A matrix job of and NW.js versions (0.64.0, 0.50.2) and node versions (10, 12, ,14, 16) that runs the NW.js test script.
- Modified `scripts/test-node-webkit.sh` so that it can now accept an NW.js version as input. This allows running the script in a GitHub Actions matrix.
- Modified `test/run.util.js` so that it does not set `--msvs_version=2015` when running in GitHub Actions. This is required because current GitHub Actions runner do not support VS Studio 2015.
- Added npm script command `test:s3` to `package.json`  that runs only the s3 tests. This is required because invoking `npx tape test/s3.test.js` on windows does not work as expected.
- Modified `test/proxy-bcrypt.test.js`. Removed uneeded CI conditionals and modified download directory setup/cleanup. Latter was required due to concurrency issues with running tests on Mac, resulting in uncatchable errors during directory removal originating from `rimraf`.
@ronilan ronilan force-pushed the Test-Create-GitHub-Actions-Test-Setup branch from b405698 to 09171e5 Compare June 30, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants