Skip to content

Conversation

@jeremymeng
Copy link
Member

No description provided.

@jeremymeng jeremymeng requested review from a team and xirzec as code owners October 31, 2025 17:36
Copilot AI review requested due to automatic review settings October 31, 2025 17:36
Copy link
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 refactors test assertions across the Form Recognizer and Document Intelligence packages to use more specific and semantically clearer assertion methods. The changes replace generic assert.ok() calls with more descriptive assertions like assert.isDefined(), assert.isNotEmpty(), and assert.include(), improving test readability and error messages.

Key changes:

  • Replace assert.ok(value) with assert.isDefined(value) for existence checks
  • Replace assert.ok(array && array.length > 0) with assert.isNotEmpty(array) for non-empty array checks
  • Replace assert.ok(message.includes(text)) with assert.include(message, text) for substring checks

Reviewed Changes

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

Show a summary per file
File Description
sdk/formrecognizer/ai-form-recognizer/test/public/training.spec.ts Updated assertions for model validation, field checks, and copy operations
sdk/formrecognizer/ai-form-recognizer/test/public/node/classifiers.spec.ts Updated assertions for classifier ID and page property checks
sdk/formrecognizer/ai-form-recognizer/test/public/node/analysis.spec.ts Updated assertions for document analysis results, pages, tables, and error messages
sdk/formrecognizer/ai-form-recognizer/test/public/browser/analysis.spec.ts Updated assertion for receipt documents check
sdk/documentintelligence/ai-document-intelligence-rest/test/public/node/training.spec.ts Updated assertions for model validation, field checks, and copy operations
sdk/documentintelligence/ai-document-intelligence-rest/test/public/node/classifiers.spec.ts Updated assertions for page properties and error handling
sdk/documentintelligence/ai-document-intelligence-rest/test/public/node/analysis.spec.ts Updated assertions for document analysis results, pages, tables, and error messages

Comment on lines +117 to 118
assert.isDefined(result.pages![0].pageNumber);
assert.isDefined(result.pages![0].angle);
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The assertion assert.isDefined(result.pages![0].angle) at line 118 is inconsistent with line 119-121 and may be incorrect. Line 118 already uses assert.isDefined for the angle property in the original code, so no change was needed. However, lines 117, 119, 120, and 121 changed from assert.ok to assert.isDefined. This inconsistency suggests that line 118 should have been left unchanged or the pattern is inconsistent with the PR's stated purpose of replacing assert.ok with more specific assertions.

Copilot uses AI. Check for mistakes.
Comment on lines +65 to 66
assert.isDefined(analyzeResult?.pages![0].pageNumber);
assert.isDefined(analyzeResult?.pages![0].angle);
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The assertion assert.isDefined(analyzeResult?.pages![0].angle) at line 66 is inconsistent with lines 67-69 and may be incorrect. Line 66 already used assert.isDefined for the angle property in the original code, so no change was needed. However, lines 65, 67, 68, and 69 changed from assert.ok to assert.isDefined. This inconsistency suggests that line 66 should have been left unchanged or the pattern is inconsistent with the PR's stated purpose of replacing assert.ok with more specific assertions.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant