Skip to content

test: Wait for the failure status in the file synchronizer pins, so they hold on Windows - #260

Open
kinyoklion wants to merge 1 commit into
rlamb/overrides-java-reject-deletedfrom
rlamb/overrides-java-windows-pins
Open

kinyoklion wants to merge 1 commit into
rlamb/overrides-java-reject-deletedfrom
rlamb/overrides-java-windows-pins

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

This resolves Finding 1 of the Windows stability assessment for the Java overrides work: plans/java-overrides-windows-test-stability.md in sdk-scratchpad.

It is stacked on #259, the top of the overrides stack, so the PRs below it do not change.

Describe the solution you've provided

#220 added FileSynchronizerReloadBehaviorTest. It pins how the legacy file data synchronizer reloads: on every file system event, reporting every failure, without retrying. On Windows, two of its tests fail every time; on Linux they pass:

  • everyFailedReloadIsReportedAndLoggedAsThePlainDescription
  • failedReloadIsNotRetriedWithoutAFileChange

Why they fail on Windows:

  • Both tests rewrite the file with Files.write, which truncates the file and then writes it.
  • On Windows that produces two WatchService keys, and the legacy synchronizer reloads after each.
  • The reload after the truncation reads an empty file. The legacy loader picks the parser by content, so the empty file goes to the YAML parser, which reads it as an empty document. The synchronizer then delivers a change set with no flags.
  • The reload after the write then reports the malformed content.
  • The tests asserted that the first result after the rewrite is that STATUS, so they received the change set instead.

A new helper, awaitStatus, consumes results until the first STATUS arrives, within the existing timeout. The three assertions that waited on the next result now use it. On Linux, the first result is still the STATUS. The pins still describe the legacy behavior they set out to pin: the failure is reported, each failure is logged as the plain description, and nothing retries without a file change.

To reproduce the Windows sequence on Linux, I made a scratch copy of the test that empties the file, waits 300 ms, and then writes the malformed content. Both pinned tests fail with the old assertions and pass with this change. The whole class passes on Linux as written. The file data, file synchronizer and file override test classes pass (144 tests), and checkstyleTest is clean. I did not run this on Windows. java-core CI runs on Linux only.

Describe alternatives you've considered

Writing the malformed content to a temporary file and moving it into place, which is one event on both platforms. That would pin a different writer than the one the tests describe.

Additional context

Not in this PR:

  • Finding 2. That reload after the truncation also shows a defect on main, outside the overrides work. On Windows, every in-place rewrite of a legacy file data source's file briefly applies an empty data set: all flags are gone and defaults are served until the next reload. The assessment recommends a minimal fix: treat an empty file as a failed load, which keeps the last data. That changes the legacy data sources, so it belongs in its own PR against main.
  • Finding 3. Build hygiene, all on main:
    • the compile tasks set no source encoding, so JDK 17 on Windows reports the box-drawing characters in three source files as unmappable;
    • the server pins common 2.6.0 and internal 1.13.0, which are not released yet.

Opened from termag


Note

Overview
Adds awaitStatus to FileSynchronizerReloadBehaviorTest so pins wait for the first STATUS result instead of the next synchronizer event. On Windows, in-place rewrites can emit a transient empty-file CHANGE_SET before the malformed-load status; the helper drains intermediate results within the existing timeout.

Updates everyFailedReloadIsReportedAndLoggedAsThePlainDescription and failedReloadIsNotRetriedWithoutAFileChange to use awaitStatus where they assert on failed reloads. Behavior under test is unchanged on Linux; assertions align with both platforms.

Reviewed by Cursor Bugbot for commit 8cda80f. Bugbot is set up for automated code reviews on this repo. Configure here.

…hey hold on Windows

On Windows a rewrite with Files.write produces two file system events, and the legacy synchronizer reloads after each. The reload after the truncation reads the empty file as an empty document and delivers a change set before the malformed content is reported. The two pins that asserted the first result was the STATUS now consume results until the STATUS arrives.
@kinyoklion
kinyoklion requested a review from a team as a code owner November 11, 2026 17:04

This branch has not been deployed

No deployments
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.

1 participant