Skip to content

Conversation

@dannimad
Copy link
Contributor

@dannimad dannimad commented Sep 26, 2025

Adding a couple of tests to frozen containers to validate expected behavior while doing certain actions like storage calls, attaching or refreshing the base snapshot.

Disable snapshot refresh when in read connection mode

Copilot AI review requested due to automatic review settings September 26, 2025 17:12
@github-actions github-actions bot added area: tests Tests to add, test infrastructure improvements, etc dependencies Pull requests that update a dependency file base: main PRs targeted against main branch labels Sep 26, 2025
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 adds negative testing to frozen containers to validate that certain operations behave as expected when performed on containers loaded from a frozen state. The tests verify that storage operations fail appropriately and that containers maintain their expected state during snapshot refresh scenarios.

Key changes:

  • Added three new test cases for frozen container behavior validation
  • Added dependencies for string manipulation and timer mocking utilities
  • Implemented tests for blob upload restrictions, attach operation limitations, and snapshot refresh behavior

Reviewed Changes

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

File Description
loadFrozenContainerFromPendingState.spec.ts Added three new test cases to validate frozen container behavior for blob uploads, attach operations, and snapshot refresh scenarios
package.json Added @fluid-internal/client-utils as a dev dependency to support string buffer operations in tests
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

@github-actions github-actions bot added the area: loader Loader related issues label Sep 29, 2025
this.storageAdapter,
offlineLoadEnabled,
this,
storageOnly,
Copy link
Contributor

Choose a reason for hiding this comment

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

you probably want to make this a getter, as the state can change

let codeLoader: CreateLoaderDefaultResults["codeLoader"];

const { urlResolver, codeDetails, codeLoader, loaderProps } = createLoader({
beforeEach(async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

please don't use before each like this. all the global vars make the test very hard to maintain and debug. if you want reuse functionally put it in a function, and call the function from the tests

@anthony-murphy
Copy link
Contributor

you should split this into two PR, one that changes refresh behavior, and one that adds tests unrelated to refresh

dannimad added a commit that referenced this pull request Oct 2, 2025
PR that exclusively adds more testing to frozen container. It adds the
same tests as #25562
solving comments on that PR by adding an initialization function and
removing the snapshot refresh change.
@dannimad dannimad closed this Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: loader Loader related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants