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

Test PR #33354

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Test PR #33354

wants to merge 1 commit into from

Conversation

ujjwalsoni1707
Copy link
Member

Packages impacted by this PR

@azure/cosmos

Issues associated with this PR

Describe the problem that is addressed by this PR

What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?

Are there test cases added in this PR? (If not, why?)

Provide a list of related PRs (if any)

Command used to generate this PR:**(Applicable only to SDK release request PRs)

Checklists

  • Added impacted package name to the issue description
  • Does this PR needs any fixes in the SDK Generator?** (If so, create an Issue in the Autorest/typescript repository and link it here)
  • Added a changelog (if necessary)

@Copilot Copilot bot review requested due to automatic review settings March 12, 2025 12:11
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 re-enables the AbortController.signal validation test for the CosmosClient by removing the skip and adjusting the timing parameter in the abort trigger.

  • Re-enabled the previously skipped test case to validate the abort signal behavior.
  • Adjusted the delay value in setTimeout for aborting the request.

it("should throw exception if aborted during the request", async function () {
const client = new CosmosClient({ endpoint, key: masterKey });
try {
const controller = new AbortController();
const signal = controller.signal;
setTimeout(() => controller.abort(), 1);
setTimeout(() => controller.abort(), 0.1);
Copy link
Preview

Copilot AI Mar 12, 2025

Choose a reason for hiding this comment

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

Using a delay of 0.1 may be problematic since setTimeout expects an integer value in milliseconds; non-integer delays might be coerced to 0, potentially causing unintended behavior in the test. Consider reverting to an integer delay value, such as 1 ms, for reliable timing.

Suggested change
setTimeout(() => controller.abort(), 0.1);
setTimeout(() => controller.abort(), 1);

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants