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

Add integration tests to Test CI worfklow #166

Merged
merged 6 commits into from
Jun 20, 2024
Merged

Add integration tests to Test CI worfklow #166

merged 6 commits into from
Jun 20, 2024

Conversation

DellaBitta
Copy link
Collaborator

@DellaBitta DellaBitta commented Jun 6, 2024

Add the integration tests to the test worfklow.
Remove one more integration test that exercises blocked content.

Note: when this is approved and ready to merge, update the GitHub main branch required tests to include the new integration tests and remove the older test name format.

@DellaBitta DellaBitta requested a review from hsubox76 June 6, 2024 15:26
@DellaBitta DellaBitta marked this pull request as ready for review June 6, 2024 15:26
@DellaBitta DellaBitta requested a review from dlarocque June 6, 2024 18:27

- name: run node iTests
env:
GEMINI_API_KEY: ${{secrets.GEMINI_API_KEY}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit:

Suggested change
GEMINI_API_KEY: ${{secrets.GEMINI_API_KEY}}
GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

- name: Use Node.js
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we're using setup-node@v3 and yarn <= 2, I think we can use caching to speed up dependency installs

   with:
        node-version: ${{ matrix.node-version }}
        cache: 'yarn'

https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md#caching-packages-data

Copy link
Collaborator

Choose a reason for hiding this comment

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

I couldn't find it, but does it say if it will upgrade to the latest version within semver, or if it finds a version in the cache that matches semver it will just use it, even if it's not the latest? I skimmed and I found a flag for managing the latest version of Node but it doesn't seem to apply to the packages. I don't know if matters that much though, just something to remember if there's any discrepancy between CI and local test runs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dlarocque Done!


- name: run web iTests
env:
GEMINI_API_KEY: ${{secrets.GEMINI_API_KEY}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit:

Suggested change
GEMINI_API_KEY: ${{secrets.GEMINI_API_KEY}}
GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

@@ -20,12 +20,10 @@ on:
branches: main

jobs:
test:
unit-test:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't forget to go into the settings and change the required tests - this changes the CI job name and the required check goes by name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, it's in the PR comments above. I won't forget!

- name: Use Node.js
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I couldn't find it, but does it say if it will upgrade to the latest version within semver, or if it finds a version in the cache that matches semver it will just use it, even if it's not the latest? I skimmed and I found a flag for managing the latest version of Node but it doesn't seem to apply to the packages. I don't know if matters that much though, just something to remember if there's any discrepancy between CI and local test runs.

env:
GEMINI_API_KEY: ${{secrets.GEMINI_API_KEY}}
run: |
cd packages/main
Copy link
Collaborator

@hsubox76 hsubox76 Jun 6, 2024

Choose a reason for hiding this comment

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

yarn --cwd packages/main test:node:integration if you want to save a line. Also for web below

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

@DellaBitta DellaBitta requested a review from dlarocque June 14, 2024 19:51
@DellaBitta DellaBitta merged commit 92662ca into main Jun 20, 2024
8 checks passed
@DellaBitta DellaBitta deleted the ddb-itests branch June 20, 2024 01:07
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.

3 participants