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

refactor: Use assert.equal() to see values when test fails #23172

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

Conversation

alexvy86
Copy link
Contributor

Description

The Includes ack'd ids in summary test sometimes fails against routerlicious but the error output only says false !== true so we can't tell the actual value of the number that is being compared to 1. This PR updates a few asserts in that file so they'll give us the actual value instead of just expected true, got false in the output when the test fails.

Reviewer Guidance

The review process is outlined on this wiki page.

@github-actions github-actions bot added area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch labels Nov 21, 2024
Comment on lines -1032 to -1035
// For some reason TSC gets it wrong - it assumes that attachState is constant and that assert above
// established it's AttachState.Detached, so this comparison is useless.
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tried removing it and nothing complained. Might have been a transient issue that a TS server restart would have gotten rid of?

@alexvy86 alexvy86 requested a review from a team November 21, 2024 16:10
Copy link
Collaborator

@msfluid-bot msfluid-bot left a comment

Choose a reason for hiding this comment

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

Code Coverage Summary

No packages impacted by the change.


Baseline commit: d94bd42
Baseline build: 309025
Happy Coding!!

Code coverage comparison check passed!!

@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: +245 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 467.24 KB 467.27 KB +35 Bytes
azureClient.js 564.01 KB 564.06 KB +49 Bytes
connectionState.js 724 Bytes 724 Bytes No change
containerRuntime.js 263.43 KB 263.45 KB +14 Bytes
fluidFramework.js 428.84 KB 428.85 KB +14 Bytes
loader.js 134.18 KB 134.19 KB +14 Bytes
map.js 42.71 KB 42.71 KB +7 Bytes
matrix.js 150.15 KB 150.16 KB +7 Bytes
odspClient.js 529.85 KB 529.89 KB +49 Bytes
odspDriver.js 97.88 KB 97.9 KB +21 Bytes
odspPrefetchSnapshot.js 42.81 KB 42.83 KB +14 Bytes
sharedString.js 166.23 KB 166.24 KB +7 Bytes
sharedTree.js 419.3 KB 419.3 KB +7 Bytes
Total Size 3.38 MB 3.38 MB +245 Bytes

Baseline commit: d94bd42

Generated by 🚫 dangerJS against cecdfe9

@alexvy86 alexvy86 requested a review from a team November 21, 2024 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants