Skip to content

Fix flaky CompactionTest: wait for async compaction complete event before asserting#4

Closed
Copilot wants to merge 28 commits intomainfrom
copilot/copilot20260312-dd-2825129-fix-test-failures
Closed

Fix flaky CompactionTest: wait for async compaction complete event before asserting#4
Copilot wants to merge 28 commits intomainfrom
copilot/copilot20260312-dd-2825129-fix-test-failures

Conversation

Copy link

Copilot AI commented Mar 12, 2026

Before the change?

testShouldTriggerCompactionWithLowThresholdAndEmitEvents was failing intermittently in the github/copilot-sdk-java CI environment with compactionCompleteCount == 0, even though compactionStartCount >= 1. The SessionCompactionCompleteEvent can arrive slightly after sendAndWait returns its session.idle signal due to async event delivery from the CLI — the test was asserting immediately without accounting for this race condition.

After the change?

CompactionTest.java — a CountDownLatch is added to explicitly wait up to 10 seconds for SessionCompactionCompleteEvent before making assertions:

var compactionCompleteLatch = new CountDownLatch(1);

session.on(event -> {
    events.add(event);
    if (event instanceof SessionCompactionCompleteEvent) {
        compactionCompleteLatch.countDown();
    }
});

// ... sendAndWait calls ...

// Wait up to 10s for async compaction complete delivery before asserting
assertTrue(compactionCompleteLatch.await(10, TimeUnit.SECONDS),
        "Should have received a compaction complete event within 10 seconds");

This is a timing robustness fix — the test logic is unchanged, it just no longer races against async event delivery. The 10-second window sits well inside the 120-second @Timeout.

The fix was applied to branch edburns/dd-2785864-pre-public-non-code-changes via cherry-pick of commit c76dbe8ce.

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • mvn spotless:apply has been run to format the code
  • mvn clean verify passes locally

Does this introduce a breaking change?

  • Yes
  • No


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

edburns and others added 26 commits March 5, 2026 19:05
modified:   .github/release.yml
modified:   .github/templates/index.html
modified:   .github/workflows/deploy-site.yml
modified:   .github/workflows/notes.template
modified:   .github/workflows/publish-maven.yml
modified:   README.md
modified:   jbang-example.java
modified:   pom.xml
modified:   src/main/java/com/github/copilot/sdk/package-info.java
modified:   src/site/markdown/cookbook/error-handling.md
modified:   src/site/markdown/cookbook/managing-local-files.md
modified:   src/site/markdown/cookbook/multiple-sessions.md
modified:   src/site/markdown/cookbook/persisting-sessions.md
modified:   src/site/markdown/cookbook/pr-visualization.md
modified:   src/site/markdown/getting-started.md
modified:   src/site/markdown/index.md
modified:   src/site/site.xml

On branch edburns/dd-2785864-pre-public-non-code-changes
modified:   .github/release.yml
modified:   .github/templates/index.html
modified:   .github/workflows/deploy-site.yml
modified:   .github/workflows/notes.template
modified:   .github/workflows/publish-maven.yml
modified:   README.md
modified:   jbang-example.java
modified:   pom.xml
modified:   src/main/java/com/github/copilot/sdk/package-info.java
modified:   src/site/markdown/cookbook/error-handling.md
modified:   src/site/markdown/cookbook/managing-local-files.md
modified:   src/site/markdown/cookbook/multiple-sessions.md
modified:   src/site/markdown/cookbook/persisting-sessions.md
modified:   src/site/markdown/cookbook/pr-visualization.md
modified:   src/site/markdown/getting-started.md
modified:   src/site/markdown/index.md
modified:   src/site/site.xml

Signed-off-by: Ed Burns <edburns@microsoft.com>
…this to be the official SDK, not a community SDK.

modified:   .github/copilot-instructions.md
modified:   .github/templates/index.html
modified:   .github/workflows/notes.template
modified:   README.md
modified:   SUPPORT.md
modified:   pom.xml
modified:   src/site/markdown/advanced.md
modified:   src/site/markdown/documentation.md
modified:   src/site/markdown/hooks.md
modified:   src/site/markdown/index.md
modified:   src/site/site.xml

Signed-off-by: Ed Burns <edburns@microsoft.com>
modified:   README.md
modified:   jbang-example.java
modified:   src/site/markdown/cookbook/error-handling.md
modified:   src/site/markdown/cookbook/managing-local-files.md
modified:   src/site/markdown/cookbook/multiple-sessions.md
modified:   src/site/markdown/cookbook/persisting-sessions.md
modified:   src/site/markdown/cookbook/pr-visualization.md

Fixed comment from @brunoborges:

> The pin snapshot version is 0.1.30, therefore all places that reference a version that is supposed to be on Maven Central, must be 0.1.29 (as if it was there already).
new file:   .github/workflows/publish-snapshot.yml

- New workflow, to publish `-SNAPSHOT` build.

modified:   .vscode/settings.json

- Always reload.

modified:   README.md

- Document `-SNAPSHOT` build.

modified:   pom.xml

- Use correct `artifactId`.

- Add `<distributionManagement />` section.
…pl is at 0.1.32.

modified:   README.md
modified:   jbang-example.java
modified:   pom.xml
modified:   src/site/markdown/cookbook/error-handling.md
modified:   src/site/markdown/cookbook/managing-local-files.md
modified:   src/site/markdown/cookbook/multiple-sessions.md
modified:   src/site/markdown/cookbook/persisting-sessions.md
modified:   src/site/markdown/cookbook/pr-visualization.md

no changes added to commit (use "git add" and/or "git commit -a")
…785864-pre-public-non-code-changes

# Conflicts:
#	README.md
#	jbang-example.java
#	pom.xml
…napshots to maven central instead of GitHub packages

modified:   .github/workflows/publish-snapshot.ym
modified:   README.md
modified:   pom.xml

- Publish to snapshots to maven central instead of GitHub packages

no changes added to commit (use "git add" and/or "git commit -a")
modified:   .github/copilot-instructions.md
modified:   README.md
modified:   src/site/markdown/advanced.md
modified:   src/site/markdown/cookbook/error-handling.md
modified:   src/site/markdown/cookbook/managing-local-files.md
modified:   src/site/markdown/cookbook/multiple-sessions.md
modified:   src/site/markdown/cookbook/persisting-sessions.md
modified:   src/site/markdown/cookbook/pr-visualization.md
modified:   src/site/markdown/documentation.md
modified:   src/site/markdown/getting-started.md
modified:   src/site/markdown/hooks.md
modified:   src/site/markdown/index.md
modified:   src/site/markdown/mcp.md

When the accessor style changed
Commit f5e9662 — February 10, 2026, authored by Bruno Borges.

"Refactor data access methods in tests to use new naming conventions"

Updated test assertions … to reflect changes in data access methods from getX() to x().

This was a massive breaking change across all 40+ event data types in com.github.copilot.sdk.events. Every public static class FooData with JavaBean getters/setters was converted to a public record FooData(...). This removed all getX() / setX() methods and replaced them with record component accessors (x()).

The commit was merged via PR #117 (feat/records-for-data-types) and shipped in v1.0.9 (the first release tag that contains it, alongside v1.0.10 and v1.0.11).

Timeline summary:

Before f5e9662: AssistantMessageData was a mutable static class with getContent(), setContent(), etc.
After f5e9662 (v1.0.9+): all event data types became Java records — content(), currentTokens(), tokenLimit(), messagesLength(), etc.
The README's Quick Start code was never updated to match — it still showed getContent() / getCurrentTokens() etc. until the fix applied in our current session.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…-getter-discrepancy

On branch edburns/dd-2824425-fix-record-vs-getter-discrepancy
Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
…tic-module-name

Set Automatic-Module-Name: com.github.copilot.sdk.java in JAR manifest
… and execute a smoke test based on the most recently published snapshot release as documented in the top level README.

new file:   src/test/prompts/PROMPT-smoke-test.md
Execute this prompt to create and execute a smoke test
Your branch is up to date with 'origin/edburns/dd-2785864-pre-public-non-code-changes'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   src/main/java/com/github/copilot/sdk/package-info.java

no changes added to commit (use "git add" and/or "git commit -a")
Copilot AI and others added 2 commits March 12, 2026 17:00
…ntDownLatch

The testShouldTriggerCompactionWithLowThresholdAndEmitEvents test was failing
with compactionCompleteCount == 0 because the SessionCompactionCompleteEvent
may arrive slightly after sendAndWait returns due to async event delivery from
the Copilot CLI. Added a CountDownLatch that waits up to 10 seconds for the
event after the third sendAndWait call completes, making the test resilient to
minor timing differences across CI environments.

Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix test failures on branch edburns/dd-2785864-pre-public-non-code-changes Fix flaky CompactionTest: wait for async compaction complete event before asserting Mar 12, 2026
Copilot AI requested a review from edburns March 12, 2026 17:08
@edburns
Copy link
Collaborator

edburns commented Mar 12, 2026

@copilot Please merge these changes into edburns/dd-2785864-pre-public-non-code-changes and close this pull request. Do not delete the topic branch. I must inspect the merge. I do not want this work to be lost until I do that,.

Copy link
Author

Copilot AI commented Mar 12, 2026

@copilot Please merge these changes into edburns/dd-2785864-pre-public-non-code-changes and close this pull request. Do not delete the topic branch. I must inspect the merge. I do ...

The agent environment only allows pushing to the PR branch (copilot/copilot20260312-dd-2825129-fix-test-failures), so I cannot directly push to edburns/dd-2785864-pre-public-non-code-changes.

The fix is entirely contained in a single commit — c76dbe8ce — which changes only src/test/java/com/github/copilot/sdk/CompactionTest.java. To apply it to your branch and close this PR, you can run:

git checkout edburns/dd-2785864-pre-public-non-code-changes
git cherry-pick c76dbe8ced3237838fdbe69a0a0e2fa5aabe67d4
git push origin edburns/dd-2785864-pre-public-non-code-changes

After that you can close this PR. I have not deleted the topic branch.

@edburns edburns closed this Mar 12, 2026
@edburns edburns deleted the copilot/copilot20260312-dd-2825129-fix-test-failures branch March 12, 2026 23:00
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.

Fix test failures on branch edburns/dd-2785864-pre-public-non-code-changes

2 participants