Skip to content

Adds NYQL-api-tests.yml #770

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
chrismulholland merged 6 commits into from
Jul 13, 2026
+9 −2
Merged

Adds NYQL-api-tests.yml #770

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
06548b7
add NYQL-api-tests.yml
chrismulholland Jul 2, 2026
bd9bab4
deleted file NYQL-api-tests.yml. added step to integration-tests.yml…
chrismulholland Jul 6, 2026
a5467e6
updated API testing section of readme
chrismulholland Jul 7, 2026
e94c11f
added env conditional to Run NYQL API tests step
chrismulholland Jul 9, 2026
35acf64
updated API testing section of readme
chrismulholland Jul 9, 2026
aca96af
copy env logic from loadConfig into nyql helpers file
chrismulholland Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/integration-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,5 @@ jobs:
env:
ENV: ${{env.ENV}}

@samanthaandrews samanthaandrews Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@chrismulholland this env var is likely qa on QA deploys and prod or production on prod deploys. You'll need to confirm the correct string. We can add some logic to decide which URL to pass in on line 47. You're already setting the url as an env var in the previously merged PR, so you'll just need to pass in the value on line 47 as a command line argument instead of in helpers.js. Lmk if this makes sense! Happy to help walk through it with you.

@chrismulholland chrismulholland Jul 9, 2026

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.

@samanthaandrews I added env.NYQL_TEST_BASE_URL to the "Run NYQL API tests" step, set via a conditional expression: production branch gets https://platform.nypl.org/api/v0.1, everything else gets https://qa-platform.nypl.org/api/v0.1.

@samanthaandrews samanthaandrews Jul 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice! Did you by chance run these locally using the prod url to make sure the tests also run and pass on that env? I'm worried they may fail if the data is different but we'll see

@chrismulholland chrismulholland Jul 9, 2026

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.

yes! both of these pass when run locally:
npm run nyql-api-test

NYQL_TEST_BASE_URL=https://platform.nypl.org/api/v0.1 npm run nyql-api-test

@charmingduchess charmingduchess Jul 10, 2026

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.

let's use the built in loadConfig method somewhere in the test set up instead https://github.com/NYPL/discovery-api/blob/main/lib/load-config.js

@chrismulholland chrismulholland Jul 10, 2026

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.

@charmingduchess the latest push takes the ENV logic from loadConfig and adds it to the nyql api tests helper file. When I used loadConfig directly I was getting KMS errors locally. Copilot suggested the help file implementation as an equivalent replacement, as it provides the ENV info needed, while avoiding KMS errors that were seen locally when adding loadConfig directly.

run: node test/integration/delivery-locations-by-barcode.test.js
- name: Run NYQL API tests
run: npm run nyql-api-test
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ npm test

### API testing

NYQL integration tests are a QA contract suite in `test/integration/nyql/`. They verify that NYQL queries return the expected results against live QA data and are not run by default during `npm test` to avoid adding latency to CI.
NYQL integration tests are a QA contract suite in `test/integration/nyql/`. They verify that NYQL queries return the expected results against live QA data and are not run by default during `npm test` to avoid adding latency to CI. They do, however, run after deploys to QA. The call to run them has been added to integration-tests.yml file.

To run them locally:

Expand Down
Loading