Skip to content

[notfication-hubs] Remove the need for BodilessMatcher in the tests with sanitizers#35447

Merged
HarshaNalluru merged 4 commits intoAzure:mainfrom
HarshaNalluru:harshan/notification-hubs-tests
Aug 4, 2025
Merged

[notfication-hubs] Remove the need for BodilessMatcher in the tests with sanitizers#35447
HarshaNalluru merged 4 commits intoAzure:mainfrom
HarshaNalluru:harshan/notification-hubs-tests

Conversation

@HarshaNalluru
Copy link
Contributor

@HarshaNalluru HarshaNalluru commented Aug 4, 2025

Packages impacted by this PR

@azure/notification-hubs

Bodiless Matcher is flaky

Bodiless Matcher doesn't always get activated even though registered as part of the test, leading to test failures in the pipeline for some jobs, not all.
I couldn't reproduce the issue locally, but there are several instances of evidence from the pipeline.

  1. Pipeline/job complaining mismatch in req body - https://dev.azure.com/azure-sdk/public/_build/results?buildId=5175819&view=logs&j=3b001400-51a2-579b-5880-362a5b0a112a&t=4fd227ba-0f36-57a1-8fce-5a18caa54ed9&l=1759
  2. Corresponding test recording - azure-sdk-assets/js/sdk/notificationhubs/notification-hubs/recordings/node/listregistrationsbytag/recording_should_list_all_registrations.json at js/notificationhubs/notification-hubs_9b371a54ae · Azure/azure-sdk-assets
  3. corresponding test that sets BodiliessMatcher -
    Noticed by @jeremymeng in the notification-hubs pipeline in JS.
    Being addressed by @scbedd at Force entrylock on matcher value azure-sdk-tools#11515

Fix for @azure/notification-hubs

  1. Remove bodiless matcher in the tests
  2. Add new sanitizers to make the timestamps constant and predictable with dummy timestamps
  3. Update recordings to have the dummy timestamps from the sanitizers

Copilot AI review requested due to automatic review settings August 4, 2025 23:04
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 removes the flaky BodilessMatcher from notification-hubs tests and replaces it with a more reliable approach using body sanitizers and fake timers. The BodilessMatcher was causing intermittent test failures in the pipeline due to inconsistent activation.

Key changes:

  • Replaced BodilessMatcher with body sanitizers that handle dynamic timestamps
  • Added fake timers in playback mode to ensure deterministic test behavior
  • Consolidated sanitizer logic into the shared createRecordedClientContext function

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
recordedClient.ts Added body sanitizers for timestamps, fake timers for playback mode, and removed browser-specific BodilessMatcher logic
listRegistrationsByTag.spec.ts Removed explicit BodilessMatcher setup
listRegistrations.spec.ts Removed explicit BodilessMatcher setup
getRegistration.spec.ts Removed explicit BodilessMatcher setup
createOrUpdateRegistration.spec.ts Removed explicit BodilessMatcher setup
assets.json Updated test recording assets tag

HarshaNalluru and others added 2 commits August 4, 2025 16:09
…dedClient.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@HarshaNalluru HarshaNalluru enabled auto-merge (squash) August 4, 2025 23:18
@HarshaNalluru HarshaNalluru merged commit 779cba7 into Azure:main Aug 4, 2025
13 checks passed
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