Skip to content

Tidy up Authorization error tests #24404

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

Merged
merged 1 commit into from
Apr 18, 2025

Conversation

CraigMacomber
Copy link
Contributor

@CraigMacomber CraigMacomber commented Apr 18, 2025

Description

Ensure these tests would fail if error was not thrown, make a bit more concise.

Cleanup done while working on #24405

Reviewer Guidance

The review process is outlined on this wiki page.

@Copilot Copilot AI review requested due to automatic review settings April 18, 2025 22:15
@github-actions github-actions bot added base: main PRs targeted against main branch area: driver Driver related issues area: odsp-driver labels Apr 18, 2025
Copy link
Contributor

@Copilot 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 tidies up error tests for authorization scenarios by ensuring tests explicitly fail if an expected error isn’t thrown and by streamlining the test logic. Key changes include cleaning up unused imports and refactoring tests to use assert.throws instead of try–catch patterns.

Comments suppressed due to low confidence (1)

packages/drivers/odsp-driver/src/test/odspError.spec.ts:331

  • [nitpick] The assertion message refers to 'realm' while the property being verified is 'tenantId', which can be confusing. Consider aligning the terminology for clarity.
assert.equal(error.tenantId, "6c482541-f706-4168-9e58-8e35a9992f58", "realm should be extracted from response");

@CraigMacomber CraigMacomber merged commit 4678e48 into microsoft:main Apr 18, 2025
35 checks passed
@CraigMacomber CraigMacomber deleted the AuthorizationError branch April 18, 2025 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: driver Driver related issues area: odsp-driver base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants