Skip to content

test: stop requesting explicit auth scopes#151

Open
corpo-iwillspeak wants to merge 2 commits intomainfrom
feature/no-explicit-auth-scopes
Open

test: stop requesting explicit auth scopes#151
corpo-iwillspeak wants to merge 2 commits intomainfrom
feature/no-explicit-auth-scopes

Conversation

@corpo-iwillspeak
Copy link
Copy Markdown
Contributor

This pull request updates the authentication configuration for both functional and unit tests by removing the scope property and replacing it with the audience property, which is now parsed using the parseAudience utility. Additionally, it introduces the grpcAddress parameter in several functional test cases and corrects an assertion in one of the tests.

Authentication configuration updates:

  • Replaced the scope property with audience, using parseAudience(process.env.VITE_ATHENA_AUDIENCE) in all ClassifierSdk instantiations in functional tests (main.functional.test.ts) and removed scope from unit tests (main.test.ts, authenticationManager.test.ts, module-import.test.ts). [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]

  • Updated the openidClient.clientCredentialsGrant mock expectation to only include audience (not scope) in unit tests (authenticationManager.test.ts).

Functional test improvements:

  • Added the grpcAddress parameter to all ClassifierSdk instantiations in functional tests to support gRPC connections (main.functional.test.ts). [1] [2] [3] [4] [5]

  • Fixed a test assertion to correctly verify that response.classifications is an array instead of a boolean (main.functional.test.ts).

Imports:

  • Added parseAudience to the import list in main.functional.test.ts for use in parsing the audience value.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the test suite to stop requesting explicit OAuth scope values, switching functional tests to pass an audience parsed via parseAudience, and extends functional tests to configure the SDK’s gRPC endpoint via grpcAddress.

Changes:

  • Remove scope from unit and functional test authentication configurations; update the auth manager unit test mock expectation accordingly.
  • Update functional tests to set audience: parseAudience(process.env.VITE_ATHENA_AUDIENCE) and pass grpcAddress from VITE_ATHENA_GRPC_ADDRESS.
  • Fix a functional test assertion to correctly verify response.classifications is an array.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/unit/module-import.test.ts Removes scope from the SDK instantiation used for import/creation verification.
tests/unit/main.test.ts Removes scope from several ClassifierSdk constructor tests.
tests/unit/authenticationManager.test.ts Removes scope from options and updates the clientCredentialsGrant expectation to match the no-scope call shape.
tests/functional/main.functional.test.ts Switches functional tests to audience via parseAudience, adds grpcAddress, and fixes an assertion on classifications.
tests/functional/e2e.functional.test.ts Removes scope from E2E test authentication config (audience remains).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 17 to +22
// This is a smoke test. You must have a running gRPC server at localhost:50051 for this to pass.
// You may want to mock the gRPC client for true unit testing.
const sdk = new ClassifierSdk({
deploymentId: process.env.VITE_ATHENA_DEPLOYMENT_ID,
affiliate: process.env.VITE_ATHENA_AFFILIATE,
grpcAddress: process.env.VITE_ATHENA_GRPC_ADDRESS,
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

The comment says the test requires a gRPC server at localhost:50051, but the SDK is now configured via grpcAddress: process.env.VITE_ATHENA_GRPC_ADDRESS (and otherwise falls back to the SDK default address). Please update the comment to reflect the actual address source (env var / default) so test setup guidance is accurate.

Copilot uses AI. Check for mistakes.
Comment on lines 259 to +265
// This is a smoke test. You must have a running gRPC server at localhost:50051 for this to pass.
// You may want to mock the gRPC client for true unit testing.
const imagePath = __dirname + '/448x448.jpg';
const sdk = new ClassifierSdk({
deploymentId: process.env.VITE_ATHENA_DEPLOYMENT_ID,
affiliate: process.env.VITE_ATHENA_AFFILIATE,
grpcAddress: process.env.VITE_ATHENA_GRPC_ADDRESS,
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

This smoke-test setup comment still references localhost:50051, but the test now uses grpcAddress from VITE_ATHENA_GRPC_ADDRESS (or the SDK default). Please update the comment so it matches how the connection target is actually chosen.

Copilot uses AI. Check for mistakes.
@snus-kin
Copy link
Copy Markdown
Member

snus-kin commented Apr 7, 2026

Dunno what yo udid the break the build but it seems unrelated to the changes in this PR.

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