Skip to content

Commit

Permalink
Flaky test fix: Run summarizer test only for local and odsp (microsof…
Browse files Browse the repository at this point in the history
…t#22153)

_"Closes the summarizing client instead of refreshing..."_ test was
failing because of the test timing out for FRS service. The code and the
test seem to work as expected. On logging timestamps it was found that
every api request takes > 1.3 seconds or more to run for FRS causing the
test to timeout.
The test is about confirming the summarizer behavior, and it doesn't
matter which service it runs against. With this PR, I am limiting the
service endpoints to be `local` and `odsp`(for covering the scenario of
single commit summaries as well, just as an extra caution).

[AB#10972](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/10972)
  • Loading branch information
pragya91 authored Aug 8, 2024
1 parent 018de36 commit e1c546c
Showing 1 changed file with 17 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,13 @@ describeCompat(
return provider.makeTestContainer(testContainerConfig);
};

beforeEach("setup", async () => {
beforeEach("setup", async function () {
provider = getTestObjectProvider({ syncSummarizer: true });
// Run these tests only for Local and ODSP drivers. Ideally we only need to run tests with local server since we are only testing the summarizer's logic independednt of the service,
// but keeping ODSP in the mix to also test code with single commit summaries. (ODSP has single commit summaries enabled by default)
if (!["local", "odsp"].includes(provider.driver.type)) {
this.skip();
}
configProvider.set("Fluid.ContainerRuntime.Test.CloseSummarizerDelayOverrideMs", 0);
});

Expand Down Expand Up @@ -98,14 +103,11 @@ describeCompat(
],
async () => {
const container = await createContainer();
const { container: summarizingContainer, summarizer } = await createSummarizer(
provider,
container,
summarizerContainerConfig,
);
const { container: summarizingContainer1, summarizer: summarizer1 } =
await createSummarizer(provider, container, summarizerContainerConfig);

// summary1
const { summaryVersion: summaryVersion1 } = await summarizeNow(summarizer);
const { summaryVersion: summaryVersion1 } = await summarizeNow(summarizer1);

// Create a second summarizer. Note that this is done before posting a summary because the server may
// delete this summary when a new one is posted.
Expand All @@ -119,9 +121,9 @@ describeCompat(
);

// summary2
await summarizeNow(summarizer);
summarizer.close();
summarizingContainer.close();
await summarizeNow(summarizer1);
summarizer1.close();
summarizingContainer1.close();

// Reconnect the second summarizer's container so that it is elected as the summarizer client.
await reconnectSummarizerToBeElected(summarizingContainer2);
Expand All @@ -131,8 +133,11 @@ describeCompat(
// tell the summarizer to process acks.
await summarizer2.run("test");

assert(summarizingContainer2.closed, "Unknown acks should close the summarizer");
assert(summarizingContainer.closed, "summarizer1 should be closed");
assert(
summarizingContainer2.closed,
"Untracked summary's acks should close the summarizer",
);
assert(summarizingContainer1.closed, "summarizer1 should be closed");
assert(!container.closed, "Original container should not be closed");
},
);
Expand Down

0 comments on commit e1c546c

Please sign in to comment.