Skip to content

SCC-5452: NYQL test foundation + first precision group (exact + identifier)#767

Merged
chrismulholland merged 22 commits into
mainfrom
SCC-5452/NYQL-api-tests-refactor
Jul 2, 2026
Merged

SCC-5452: NYQL test foundation + first precision group (exact + identifier)#767
chrismulholland merged 22 commits into
mainfrom
SCC-5452/NYQL-api-tests-refactor

Conversation

@chrismulholland

Copy link
Copy Markdown
Contributor

Summary

This PR adds the NYQL integration test foundation and first precision test group for PR 1.

Includes

  • Added test/integration/nyql/helpers.js with NYQL_TEST_BASE_URL override and shared helper(s).
  • Added test/integration/nyql/nyql-precision.test.js with the initial precision checks:
    • exact call number (single result)
    • call number all-contain behavior
    • exact identifier (res:id)
  • Updated package.json:
    • adds npm run nyql-api-test
    • keeps NYQL tests out of default npm test
    • keeps NYQL files linted
  • Updated README.md with:
    • how to run npm run nyql-api-test
    • NYQL_TEST_BASE_URL override usage

Validation

  • npm run nyql-api-test passes (3 passing)

Scope Note

  • CI workflow changes were intentionally removed from this PR and will be added in PR 2.

@samanthaandrews samanthaandrews left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Chris, this is great work. So much easier to review and really demonstrates your ability to break down the work into small pieces.

What's improved:

  • Linting is enabled which makes all of the code conform to the rest of the repo standards. There's no unused expressions or little things that we need to clean up in the PR because the linter took care of those warnings. Awesome.
  • baseUrl is in an env var with QA url as the default defined in helpers.js so we don't have to repeat it throughout the tests.
  • No stray console.logs or commented out blocks. The code is easy to read so the comments just aren't necessary, which is great.
  • Explicitly explaining in the PR description that you'll make the CI changes in PR 2

I just have a few small suggested changes that should be easy to accommodate. Feel free to re-request review from me when you're ready.

Comment thread test/integration/nyql/helpers.js
Comment thread test/integration/nyql/nyql-precision.test.js Outdated
Comment thread test/integration/nyql/nyql-precision.test.js Outdated
Comment thread test/integration/nyql/nyql-precision.test.js Outdated
Comment thread test/integration/nyql/nyql-precision.test.js Outdated
// These are some tests to verify that certain NYQL queries are returning results with the expected precision, especially for fields like call numbers where we want to ensure that the query is matching the intended values and not over- or under-matching.

describe('Discovery API - NYQL precision tests', function () {
this.timeout(30000)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Let's import and use the TIMEOUT from helpers.js here

@samanthaandrews samanthaandrews left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good on my end. Go ahead and request review from @danamansana and optionally @nonword and @charmingduchess . For context, Chris is breaking down the NYQL api test PRs into smaller pieces. The intention with this foundational PR is to merge it into main and add the rest of the test PRs on top of it (there may be 6+). They should all be pretty small and easy to review.

@danamansana danamansana left a comment

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.

Looks like a good start!

@chrismulholland
chrismulholland merged commit 60f15cb into main Jul 2, 2026
4 checks passed
@chrismulholland
chrismulholland deleted the SCC-5452/NYQL-api-tests-refactor branch July 2, 2026 16:18
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.

4 participants