test: introduce headless E2E test suites (domain, controls, shell)#2017
Open
yuto-trd wants to merge 12 commits into
Open
test: introduce headless E2E test suites (domain, controls, shell)#2017yuto-trd wants to merge 12 commits into
yuto-trd wants to merge 12 commits into
Conversation
Add the first end-to-end / headless-UI test infrastructure for Beutl using Avalonia.Headless.NUnit (runs on headless CI without xvfb or a GPU). - tests/Beutl.Testing.Headless: shared support library (BEUTL_HOME temp isolation, dispatcher pump / visual-tree helpers) used by every E2E suite. - tests/Beutl.E2ETests: Phase A suite referencing only Beutl.* libraries (not the src/Beutl app exe). TestApp uses Skia headless rendering (UseHeadlessDrawing = false) so real FluentAvalonia/Beutl.Controls templates (FontIcon glyphs, text) inflate and render headlessly. - Directory.Packages.props: pin Avalonia.Headless / Avalonia.Headless.NUnit / Avalonia.Skia to 11.3.17 (matching Avalonia). - Beutl.slnx: register both projects under /Tests/ so CI discovers them. Validates the harness end-to-end: a theme-independent layout canary plus a BooleanEditor template-application + value round-trip both pass headlessly.
Add UI-less end-to-end scenario tests under tests/Beutl.E2ETests/Scenarios that drive the real editing/persistence pipeline (Scene/Project serialization, the element edit services, HistoryManager undo/redo, and animation/keyframe round-trips). A local SceneHistoryHarness replicates the Scene + history + observer wiring without referencing the Beutl.UnitTests assembly.
Inflate real PropertyEditor controls in the headless Skia harness and assert the user-input to Value flow: simulate clicks, keystrokes and mouse-wheel gestures, and verify ValueChanged / ValueConfirmed routed events fire with the expected values. Covers BooleanEditor, StringEditor, TimeSpanEditor (incl. validation), NumberEditor<int/float/double>, Vector2/3/4Editor, RelativePointEditor, RelativeRectEditor, ColorComponentsEditor, AlignmentX/YEditor, EnumEditor, RationalEditor, AutoCompleteStringEditor, and ColorEditor. ColorEditor uses a programmatic Value round-trip because its only input gesture opens a flyout that does not open deterministically headless.
- Add [TestFixture] to all new E2E test classes (tests/CLAUDE.md convention). - Standardise dispatcher pumping on HeadlessTestHelpers.Settle() (two-pass) instead of single-pass Dispatcher.UIThread.RunJobs() to avoid pre-layout assertion fragility. - Make ColorEditor's programmatic round-trip test honest: drop the unused ValueChanged capture (the flyout-only edit path is unreachable headless) and rename it to reflect that it only covers the DirectProperty round-trip. - Pre-create scene subdirectories in all ProjectPersistence tests so they no longer depend on the undocumented recursive-reference-save directory behaviour. - Note SceneHistoryHarness is a deliberate standalone copy (the E2E project must not reference the Beutl.UnitTests assembly).
Introduce tests/Beutl.HeadlessUITests, the Phase B shell E2E suite that drives the real Beutl application shell headlessly (Avalonia.Headless.NUnit + Skia). A minimal TestApp (deriving from Application, NOT the production App) reproduces App.RegisterServices' service wiring -- property-editor / notification / tutorial handlers, the AppHelper context-command delegate, library & node registrars, and the primitive-extension load -- WITHOUT App.Initialize's RunStartupTask (which does auth / update-check / last-project-restore network and disk I/O). Global registries are populated once behind a static guard because the headless session re-runs RegisterServices per test. This intentionally changes the convention that no test references src/Beutl (.claude/rules/csharp.md): driving the shell orchestration types (ProjectService, EditorService, MainViewModel, EditViewModel) end-to-end requires them, and they live in the app exe. The GPL/MIT boundary is preserved -- src/Beutl references Beutl.FFmpegWorker with ReferenceOutputAssembly=false, so the worker never enters the test compile closure. Three InternalsVisibleTo grants (Beutl, Beutl.Core, Beutl.Extensibility -> Beutl.HeadlessUITests) let the harness set the internal service handlers the app sets from inside its own assembly. Smoke tests cover create-project disk round-trip, scene-editor tab activation, and MainViewModel open/close state.
…xception Record that tests/Beutl.HeadlessUITests is the one test project allowed to reference src/Beutl (for shell E2E), update tests/CLAUDE.md with the headless E2E conventions (Avalonia.Headless.NUnit, [AvaloniaTest], Skia rendering, reset-in-body global state, BEUTL_HOME isolation, GPU/export gating), and note both E2E suites in AGENTS.md.
…blic NotificationService.Handler and TutorialService.Current had internal setters, so only the Beutl app assembly could register the handlers an external host needs to bootstrap the services. Their getters and interfaces (INotificationServiceHandler / ITutorialService) are already public and designed for substitution; the write-once '??=' guard provides the same init-order safety the internal modifier did. Making the setters public lets any host -- the new headless shell E2E suite, an alternative shell, an automation tool -- register them, and removes the Beutl.Core InternalsVisibleTo grant that the E2E harness otherwise needed (the property-editor handler stays internal, so the Beutl / Beutl.Extensibility grants remain). Non-breaking: the existing App.RegisterServices call site is unchanged.
… shell Phase B1/B2 end-to-end tests that exercise the production orchestration types in src/Beutl through the headless harness (TestApp + Avalonia headless Skia). B1 (GPU-free, all pass): - OpenProjectTests: create -> close -> OpenProject a persisted .bproj, asserting it reloads, IsOpened, the scene round-trips, and project variables survive. - EditorWorkflowTests: reach the real EditViewModel via the activated editor tab, add an element through IElementAdder, edit a property, and assert Scene.Children plus HistoryManager undo/redo (also through IKnownEditorCommands). - SaveRoundTripTests: mutate through the editor, save via the editor's OnSave path, reopen, and assert the mutation survived on disk. - MainViewModelExtensionTests: assert ToolTabExtensions / EditorExtensions are populated with the built-in Timeline tool tab and scene editor. - ShellViewTests: host the real EditView in a headless window, asserting it inflates, lays out, and captures a rendered frame. The full MainView is not hosted (needs app-only dock/title-bar resources); EditView is the largest real shell view that works headless. B2 (gated): - PreviewRenderTests: render a real preview frame through SceneRenderer, gated on GPU availability (GpuTestGate mirrors VulkanTestEnvironment); runs on MoltenVK. - ExportTests: construct/validate the OutputViewModel (defaults, supersample buffer-limit warning, CanEncode) without spawning the FFmpeg worker. The full end-to-end export is documented as blocked headless (native-codec load hangs, worker assembly absent from test output, IPC deadlocks the headless dispatcher). TestReset centralizes the on-UI-thread global-state reset and disposes open editor tabs so their FileSystemWatchers do not fire into a disposed view model in a later test. The harness (csproj, TestApp.axaml) is unchanged.
- Make TestReset.ResetShellAsync async and await EditorService.CloseTabItem instead of blocking the UI thread with .GetAwaiter().GetResult(); blocking would deadlock against player-pause callbacks posted to the dispatcher if a test ever leaves playback running. All callers await it. - Mark GpuTestGate.s_initialized volatile so the double-checked-locking fast path is safe under weak (ARM64) memory ordering. - Add [TestFixture] to all HeadlessUITests classes (tests/CLAUDE.md convention). - Note that the editor ZIndex tests intentionally exercise the observer->history pipeline rather than a specific edit service.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds shared headless test infrastructure, updates service access for tests, wires two test applications, and expands E2E/headless UI coverage for editors, scene history, persistence, and shell workflows. ChangesHeadless Test Rollout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces Beutl’s first headless end-to-end (E2E) UI testing infrastructure using Avalonia.Headless.NUnit, adding two new E2E suites plus a shared headless-testing helper library. It also relaxes two core service registration points (Notification/Tutorial) to be publicly settable so external hosts (like the headless harness) can register handlers.
Changes:
- Add
Beutl.Testing.Headlesssupport library (BEUTL_HOME isolation + dispatcher/visual-tree helpers). - Add
Beutl.E2ETests(library-level + headless property-editor tests) andBeutl.HeadlessUITests(real app-shell orchestration) projects and wire them into the solution. - Make
NotificationService.HandlerandTutorialService.Currentsetters public to allow external hosts to register handlers.
Reviewed changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/CLAUDE.md | Document new headless E2E test conventions and project roles. |
| tests/Beutl.Testing.Headless/HeadlessTestHelpers.cs | Add dispatcher settle/render helpers + visual-tree search. |
| tests/Beutl.Testing.Headless/BeutlHomeIsolation.cs | Add per-assembly BEUTL_HOME temp isolation helper. |
| tests/Beutl.Testing.Headless/Beutl.Testing.Headless.csproj | Add shared headless helper project. |
| tests/Beutl.HeadlessUITests/TestReset.cs | Add UI-thread-safe global shell state reset for tests. |
| tests/Beutl.HeadlessUITests/TestAppBuilder.cs | Configure headless Avalonia app builder for shell tests. |
| tests/Beutl.HeadlessUITests/TestApp.axaml.cs | Implement minimal headless Application registering shell services/extensions. |
| tests/Beutl.HeadlessUITests/TestApp.axaml | Register styles needed to inflate shell views headlessly. |
| tests/Beutl.HeadlessUITests/ShellViewTests.cs | Add headless inflation/layout and frame-capture tests for EditView. |
| tests/Beutl.HeadlessUITests/ShellSmokeTests.cs | Add shell-level smoke tests (create/open project, tab activation, VM state). |
| tests/Beutl.HeadlessUITests/SaveRoundTripTests.cs | Add save + reopen persistence tests through real editor VM. |
| tests/Beutl.HeadlessUITests/PreviewRenderTests.cs | Add GPU-gated preview render tests via SceneRenderer. |
| tests/Beutl.HeadlessUITests/OpenProjectTests.cs | Add open-project round-trip tests for persisted .bproj. |
| tests/Beutl.HeadlessUITests/MainViewModelExtensionTests.cs | Validate extensions are populated in MainViewModel. |
| tests/Beutl.HeadlessUITests/GpuTestGate.cs | Add reusable GPU availability probe + self-skip helper. |
| tests/Beutl.HeadlessUITests/ExportTests.cs | Add non-worker export VM validation tests + documented worker blockers. |
| tests/Beutl.HeadlessUITests/EditorWorkflowTests.cs | Add shell orchestration workflow tests (edit, undo/redo, commands). |
| tests/Beutl.HeadlessUITests/Beutl.HeadlessUITests.csproj | Add headless shell E2E test project + app reference. |
| tests/Beutl.HeadlessUITests/AssemblySetUp.cs | Ensure BEUTL_HOME isolation is enabled for shell E2E assembly. |
| tests/Beutl.E2ETests/TestInfrastructure/SceneHistoryHarness.cs | Add standalone history wiring harness for domain scenarios. |
| tests/Beutl.E2ETests/TestAppBuilder.cs | Configure headless Avalonia app builder for library-level tests. |
| tests/Beutl.E2ETests/TestApp.axaml.cs | Add minimal headless Application for E2E test hosting. |
| tests/Beutl.E2ETests/TestApp.axaml | Register core styles for property editor inflation. |
| tests/Beutl.E2ETests/Scenarios/UndoRedoTests.cs | Add domain-level undo/redo scenario coverage. |
| tests/Beutl.E2ETests/Scenarios/SceneRoundTripTests.cs | Add scene/element persistence round-trip tests. |
| tests/Beutl.E2ETests/Scenarios/ProjectPersistenceTests.cs | Add project persistence round-trip tests + resave scenario. |
| tests/Beutl.E2ETests/Scenarios/ElementEditingPipelineTests.cs | Add editing-pipeline scenario tests (move/resize/split/group/etc). |
| tests/Beutl.E2ETests/Scenarios/ClipboardPipelineTests.cs | Add clipboard pipeline tests using a fake clipboard gateway. |
| tests/Beutl.E2ETests/Scenarios/AnimationRoundTripTests.cs | Add animation/keyframe serialization round-trip tests. |
| tests/Beutl.E2ETests/HeadlessHarnessTests.cs | Add basic headless-host smoke tests for window/control layout. |
| tests/Beutl.E2ETests/Controls/VectorEditorTests.cs | Add headless property editor interaction tests (vectors/relative point). |
| tests/Beutl.E2ETests/Controls/TimeSpanEditorTests.cs | Add time-span editor validation + confirm behavior tests. |
| tests/Beutl.E2ETests/Controls/StringEditorTests.cs | Add string editor change/confirm tests. |
| tests/Beutl.E2ETests/Controls/RelativeRectEditorTests.cs | Add relative-rect editor interaction tests. |
| tests/Beutl.E2ETests/Controls/RationalEditorTests.cs | Add rational editor parsing/validation tests. |
| tests/Beutl.E2ETests/Controls/NumberEditorTests.cs | Add numeric editor typing/wheel/validation tests. |
| tests/Beutl.E2ETests/Controls/EnumEditorTests.cs | Add enum editor selection/confirm tests. |
| tests/Beutl.E2ETests/Controls/EditorTestHost.cs | Add reusable host that simulates input in a headless window. |
| tests/Beutl.E2ETests/Controls/ColorEditorTests.cs | Add limited color editor template/direct-property tests. |
| tests/Beutl.E2ETests/Controls/ColorComponentsEditorTests.cs | Add RGB component editor typing tests. |
| tests/Beutl.E2ETests/Controls/BooleanEditorTests.cs | Add boolean editor click + programmatic update tests. |
| tests/Beutl.E2ETests/Controls/AutoCompleteStringEditorTests.cs | Add autocomplete editor typing + confirm tests. |
| tests/Beutl.E2ETests/Controls/AlignmentEditorTests.cs | Add alignment editor click + binding reflection tests. |
| tests/Beutl.E2ETests/Beutl.E2ETests.csproj | Add library-level E2E test project and references. |
| tests/Beutl.E2ETests/AssemblySetUp.cs | Ensure BEUTL_HOME isolation is enabled for E2E assembly. |
| src/Beutl/Properties/AssemblyInfo.cs | Add IVT access for shell E2E to app internals. |
| src/Beutl.Extensibility/Properties/AssemblyInfo.cs | Add IVT access for shell E2E to extensibility internals. |
| src/Beutl.Core/Services/Tutorials/TutorialService.cs | Make TutorialService.Current setter public for external host registration. |
| src/Beutl.Core/Services/INotificationService.cs | Make NotificationService.Handler setter public for external host registration. |
| Directory.Packages.props | Add central package versions for Avalonia headless/skia test packages. |
| Beutl.slnx | Include the new test projects in the solution. |
| AGENTS.md | Document the new headless E2E test suites at the repo overview level. |
| .claude/rules/csharp.md | Document the src/Beutl test-reference exception for shell E2E. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (2)
tests/Beutl.E2ETests/TestInfrastructure/SceneHistoryHarness.cs (1)
7-15: 📐 Maintainability & Code Quality | 🔵 TrivialMove
SceneHistoryHarnessintotests/Beutl.Testing.Headless
Beutl.E2ETestsalready references the shared test-support project, so this harness can live there and be reused by both test suites instead of keeping duplicate copies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Beutl.E2ETests/TestInfrastructure/SceneHistoryHarness.cs` around lines 7 - 15, Move the shared SceneHistoryHarness out of the E2E test project and into the Beutl.Testing.Headless test-support project so both suites can reuse the same implementation. Update the SceneHistoryHarness type and its related wiring (including the Scene-rooted setup with OperationSequenceGenerator, HistoryManager, and CoreObjectOperationObserver) in the shared project, then have the E2E tests reference that shared harness instead of keeping a standalone copy.Source: Learnings
src/Beutl.Core/Services/INotificationService.cs (1)
7-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePublic setter silently ignores subsequent assignments.
Now that
setis public, callers outside the assembly can assignHandler, buts_handler ??= valuemeans any assignment after the first is silently dropped (no exception, no replacement). For an external host this can be surprising during teardown/re-registration. Consider documenting the one-time semantics on the property, or returning/throwing on re-assignment so misuse is visible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Beutl.Core/Services/INotificationService.cs` around lines 7 - 11, The public Handler setter in INotificationService currently allows external assignment but silently ignores any re-assignment because it uses one-time initialization semantics. Update the property behavior so misuse is visible: either document clearly that Handler can only be set once, or change the setter to explicitly reject subsequent assignments by throwing or otherwise signaling failure. Keep the fix centered on INotificationService.Handler and its backing field s_handler so callers do not assume replacement is supported.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/Beutl.E2ETests/Controls/AlignmentEditorTests.cs`:
- Around line 12-61: Add the required singleton reset at the start of each
`[AvaloniaTest]` in `AlignmentEditorTests` so test state is isolated per run;
update the test bodies for
`AlignmentX_clicking_a_radio_button_sets_value_and_raises_events`,
`AlignmentY_clicking_a_radio_button_sets_value`, and
`AlignmentX_setting_value_checks_the_matching_radio_button` to reset shared
globals like `ProjectService.Current` and `EditorService.Current` before
creating the `AlignmentXEditor` or `AlignmentYEditor` and `EditorTestHost`
instances, following the existing coding guideline and avoiding any
`[SetUp]`/`[TearDown]` usage.
In `@tests/Beutl.E2ETests/Controls/AutoCompleteStringEditorTests.cs`:
- Around line 10-52: The two `[AvaloniaTest]` methods in
`AutoCompleteStringEditorTests` need to reset shared singleton state at the
start of each test to avoid leaking app state between cases. Update
`Typing_into_the_autocomplete_box_updates_text_and_raises_value_changed` and
`Focus_loss_after_an_edit_confirms` to perform the required singleton resets
inside each test body before creating `AutoCompleteStringEditor` or
`EditorTestHost`, following the test guidance for shared globals like
`ProjectService.Current` and `EditorService.Current`.
In `@tests/Beutl.E2ETests/Controls/BooleanEditorTests.cs`:
- Around line 11-51: The BooleanEditorTests[AvaloniaTest] methods are relying on
shared global state that may leak across tests; reset the relevant singletons at
the start of each test body before creating the BooleanEditor or EditorTestHost.
Apply this in both Clicking_the_checkbox_flips_value_and_raises_events and
Setting_value_programmatically_updates_the_checkbox, following the guideline to
reset shared globals like ProjectService.Current and EditorService.Current
inside each test rather than in setup/teardown.
In `@tests/Beutl.E2ETests/Controls/ColorComponentsEditorTests.cs`:
- Around line 10-26: Reset the shared singletons at the start of this Avalonia
test to keep it isolated: in Typing_rgb_components_recomputes_the_color, assign
both ProjectService.Current and EditorService.Current to null before creating
the ColorComponentsEditor and EditorTestHost. Keep the reset inside the test
body rather than moving it to SetUp or TearDown, so this headless test does not
leak state into other tests.
In `@tests/Beutl.E2ETests/Controls/ColorEditorTests.cs`:
- Around line 15-36: Add the required singleton reset at the start of each
AvaloniaTest in ColorEditorTests so each test runs with a clean global state.
Update both Template_applies_and_exposes_the_picker_button and
Value_assignment_round_trips_through_the_direct_property to reset shared globals
like ProjectService.Current and EditorService.Current before creating the
ColorEditor or EditorTestHost, rather than relying on shared state from other
tests.
In `@tests/Beutl.E2ETests/Controls/EditorTestHost.cs`:
- Around line 15-30: Add deterministic teardown to EditorTestHost so the Window
created in the constructor is always closed/disposed after each test. Introduce
a cleanup path on the EditorTestHost<TEditor> type (for example by implementing
IDisposable or an explicit dispose method) that closes Window and releases any
associated state, and make sure tests call it even if they fail. Use the
existing EditorTestHost, Window, and HeadlessTestHelpers.Settle setup as the
place to wire in the teardown.
In `@tests/Beutl.E2ETests/Controls/EnumEditorTests.cs`:
- Around line 25-53: The two [AvaloniaTest] methods in EnumEditorTests are
missing the required test-isolation reset for shared singletons. At the start of
each test body, reset both ProjectService.Current and EditorService.Current
before creating the EnumEditor or EditorTestHost, so each test runs with a clean
global state. Keep the reset local to the tests themselves rather than moving it
to shared setup/teardown.
In `@tests/Beutl.E2ETests/Controls/NumberEditorTests.cs`:
- Around line 10-82: These Avalonia tests are missing explicit cleanup of shared
global state, so each test should reset ProjectService.Current and
EditorService.Current at the start of the test body for isolation. Update every
[AvaloniaTest] in NumberEditorTests to perform the singleton reset before
creating the NumberEditor or EditorTestHost, following the existing testing
pattern and keeping the reset local to each test rather than using
SetUp/TearDown.
- Around line 27-36: The float typing test is locale-sensitive because
NumberEditor<T> parses using CultureInfo.CurrentUICulture, so the hardcoded
input can fail on comma-based locales. Update Typing_a_float_updates_value in
NumberEditorTests to either set the test culture explicitly or generate the
numeric string from the current UI culture before calling host.TypeInto, and
keep the assertion aligned with the culture-aware expected value.
In `@tests/Beutl.E2ETests/Controls/RationalEditorTests.cs`:
- Around line 10-54: Each [AvaloniaTest] in RationalEditorTests is missing the
required per-test reset of shared singletons, which can make results
order-dependent. Add the reset at the start of each test body in this class,
before creating RationalEditor or EditorTestHost, and do not rely on shared
state from previous tests. Use the existing test methods
Typing_a_fraction_parses_into_value, Focus_loss_after_an_edit_confirms, and
Invalid_text_raises_a_validation_error_and_keeps_value as the places to apply
the reset for globals like ProjectService.Current and EditorService.Current.
In `@tests/Beutl.E2ETests/Controls/RelativeRectEditorTests.cs`:
- Around line 11-42: The RelativeRectEditorTests Avalonia tests are missing the
required singleton cleanup at the start of each test body. In both
`[AvaloniaTest]` methods, add the mandated reset of shared globals like
`ProjectService.Current` and `EditorService.Current` before creating
`RelativeRectEditor` or `EditorTestHost<RelativeRectEditor>`, and do it inside
each test rather than in shared setup/teardown.
In `@tests/Beutl.E2ETests/Controls/StringEditorTests.cs`:
- Around line 11-61: The StringEditorTests AvaloniaTest methods are missing the
required singleton reset at the start of each test body. Add the
ProjectService.Current and EditorService.Current resets at the beginning of each
of the three test methods in StringEditorTests so each test starts from a clean
global state; use the existing test methods
Typing_updates_text_and_raises_value_changed_while_focused,
Losing_focus_after_an_edit_raises_value_confirmed, and
Focus_loss_without_a_change_does_not_confirm as the insertion points.
In `@tests/Beutl.E2ETests/Controls/TimeSpanEditorTests.cs`:
- Around line 10-72: Each `[AvaloniaTest]` in `TimeSpanEditorTests` must reset
shared globals at the start of the test body to avoid order-dependent flakiness.
Update every test method in this class to perform the required singleton reset
before creating `TimeSpanEditor` or `EditorTestHost<TimeSpanEditor>`, following
the same pattern used elsewhere for `ProjectService.Current` and
`EditorService.Current`. Keep the reset inside each test method rather than
moving it to `SetUp` or `TearDown`.
In `@tests/Beutl.E2ETests/Controls/VectorEditorTests.cs`:
- Around line 11-107: Each `[AvaloniaTest]` in `VectorEditorTests` must reset
shared global singletons at the start of the test body to avoid cross-test
contamination. Update the existing test methods like
`Vector2_editing_one_component_updates_only_that_component`,
`Vector2_focus_loss_confirms_the_composed_pair`,
`Vector2_wheel_changes_the_hovered_component`,
`Vector3_editing_all_three_components_composes_the_value`,
`Vector4_editing_all_four_components_composes_the_value`, and
`RelativePoint_editing_components_confirms_a_composed_relative_point` to
initialize the required globals before creating the editor or host. Do not move
this into shared setup/teardown; keep the reset inside each `[AvaloniaTest]`
body as required by the guideline.
In `@tests/Beutl.E2ETests/HeadlessHarnessTests.cs`:
- Around line 11-39: These Avalonia tests should reset shared singleton state at
the start of each `[AvaloniaTest]` body so state does not leak between runs.
Update the `Headless_session_lays_out_a_control` and
`BooleanEditor_applies_template_and_round_trips_value` tests in
`HeadlessHarnessTests` to clear/reset the relevant globals such as
`ProjectService.Current` and `EditorService.Current` before creating the
`Window` or control, following the existing per-test pattern rather than relying
on shared setup/teardown.
- Around line 14-17: The tests leave windows open after calling window.Show(),
which can leak UI state into later runs. Update the affected test methods in
HeadlessHarnessTests to explicitly close the Window after
HeadlessTestHelpers.Settle() (or use a teardown/disposal path) so each test
cleans up its shown window instance.
In `@tests/Beutl.HeadlessUITests/MainViewModelExtensionTests.cs`:
- Around line 34-40: The test in MainViewModelExtensionTests disposes
MainViewModel only after the assertions, so a failed assertion can skip cleanup
and leak state into later tests. Update the test to ensure vm.Dispose() always
runs by wrapping the assertions around MainViewModel in a try/finally or
equivalent disposal pattern, using the MainViewModel symbol as the cleanup
target.
- Around line 13-33: Reset shared globals at the start of each `[AvaloniaTest]`
in `MainViewModelExtensionTests`, since `SharedMainViewModel` and the extension
assertions can be affected by state carried over from previous tests. Add the
required per-test reset at the beginning of each test body, following the
project guideline to reset singletons like `ProjectService.Current` and
`EditorService.Current` before using `SharedMainViewModel`, `ToolTabExtensions`,
or `EditorExtensions`.
In `@tests/Beutl.HeadlessUITests/ShellSmokeTests.cs`:
- Around line 70-73: The ShellSmokeTests “open and close” scenario is asserting
the wrong state transition because vm.Dispose() is not the same as closing the
project. Update the test to use ProjectService.Current.CloseProject() in the
open/close flow, then settle and assert vm.IsProjectOpened on the ViewModel so
the check matches the actual close path. Use the existing ShellSmokeTests and
vm.IsProjectOpened references to keep the change localized.
---
Nitpick comments:
In `@src/Beutl.Core/Services/INotificationService.cs`:
- Around line 7-11: The public Handler setter in INotificationService currently
allows external assignment but silently ignores any re-assignment because it
uses one-time initialization semantics. Update the property behavior so misuse
is visible: either document clearly that Handler can only be set once, or change
the setter to explicitly reject subsequent assignments by throwing or otherwise
signaling failure. Keep the fix centered on INotificationService.Handler and its
backing field s_handler so callers do not assume replacement is supported.
In `@tests/Beutl.E2ETests/TestInfrastructure/SceneHistoryHarness.cs`:
- Around line 7-15: Move the shared SceneHistoryHarness out of the E2E test
project and into the Beutl.Testing.Headless test-support project so both suites
can reuse the same implementation. Update the SceneHistoryHarness type and its
related wiring (including the Scene-rooted setup with
OperationSequenceGenerator, HistoryManager, and CoreObjectOperationObserver) in
the shared project, then have the E2E tests reference that shared harness
instead of keeping a standalone copy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: acc19007-c780-4c91-97cf-dfd0b87ca03e
📒 Files selected for processing (53)
.claude/rules/csharp.mdAGENTS.mdBeutl.slnxDirectory.Packages.propssrc/Beutl.Core/Services/INotificationService.cssrc/Beutl.Core/Services/Tutorials/TutorialService.cssrc/Beutl.Extensibility/Properties/AssemblyInfo.cssrc/Beutl/Properties/AssemblyInfo.cstests/Beutl.E2ETests/AssemblySetUp.cstests/Beutl.E2ETests/Beutl.E2ETests.csprojtests/Beutl.E2ETests/Controls/AlignmentEditorTests.cstests/Beutl.E2ETests/Controls/AutoCompleteStringEditorTests.cstests/Beutl.E2ETests/Controls/BooleanEditorTests.cstests/Beutl.E2ETests/Controls/ColorComponentsEditorTests.cstests/Beutl.E2ETests/Controls/ColorEditorTests.cstests/Beutl.E2ETests/Controls/EditorTestHost.cstests/Beutl.E2ETests/Controls/EnumEditorTests.cstests/Beutl.E2ETests/Controls/NumberEditorTests.cstests/Beutl.E2ETests/Controls/RationalEditorTests.cstests/Beutl.E2ETests/Controls/RelativeRectEditorTests.cstests/Beutl.E2ETests/Controls/StringEditorTests.cstests/Beutl.E2ETests/Controls/TimeSpanEditorTests.cstests/Beutl.E2ETests/Controls/VectorEditorTests.cstests/Beutl.E2ETests/HeadlessHarnessTests.cstests/Beutl.E2ETests/Scenarios/AnimationRoundTripTests.cstests/Beutl.E2ETests/Scenarios/ClipboardPipelineTests.cstests/Beutl.E2ETests/Scenarios/ElementEditingPipelineTests.cstests/Beutl.E2ETests/Scenarios/ProjectPersistenceTests.cstests/Beutl.E2ETests/Scenarios/SceneRoundTripTests.cstests/Beutl.E2ETests/Scenarios/UndoRedoTests.cstests/Beutl.E2ETests/TestApp.axamltests/Beutl.E2ETests/TestApp.axaml.cstests/Beutl.E2ETests/TestAppBuilder.cstests/Beutl.E2ETests/TestInfrastructure/SceneHistoryHarness.cstests/Beutl.HeadlessUITests/AssemblySetUp.cstests/Beutl.HeadlessUITests/Beutl.HeadlessUITests.csprojtests/Beutl.HeadlessUITests/EditorWorkflowTests.cstests/Beutl.HeadlessUITests/ExportTests.cstests/Beutl.HeadlessUITests/GpuTestGate.cstests/Beutl.HeadlessUITests/MainViewModelExtensionTests.cstests/Beutl.HeadlessUITests/OpenProjectTests.cstests/Beutl.HeadlessUITests/PreviewRenderTests.cstests/Beutl.HeadlessUITests/SaveRoundTripTests.cstests/Beutl.HeadlessUITests/ShellSmokeTests.cstests/Beutl.HeadlessUITests/ShellViewTests.cstests/Beutl.HeadlessUITests/TestApp.axamltests/Beutl.HeadlessUITests/TestApp.axaml.cstests/Beutl.HeadlessUITests/TestAppBuilder.cstests/Beutl.HeadlessUITests/TestReset.cstests/Beutl.Testing.Headless/Beutl.Testing.Headless.csprojtests/Beutl.Testing.Headless/BeutlHomeIsolation.cstests/Beutl.Testing.Headless/HeadlessTestHelpers.cstests/CLAUDE.md
A DirectoryWatcher FileSystemWatcher event can post a RefreshHomeView after the owning FileBrowserTab view model (and its MediaFileSearcher) has been disposed. SearchAsync then called Cancel() on the already-disposed CancellationTokenSource, throwing ObjectDisposedException onto the dispatcher. Track a _disposed flag and return early from SearchAsync so a late watcher event is ignored instead of crashing. Surfaced as intermittent failures in the headless shell E2E suite when editor tabs (which open the file-browser tool tab) are torn down between tests.
- Harden public service-handler setters (NotificationService.Handler, TutorialService.Current) with ArgumentNullException.ThrowIfNull (Copilot). - Make NumberEditor float/double tests culture-robust by formatting the typed value with CultureInfo.CurrentUICulture, so they don't fail on comma locales. - Give EditorTestHost a deterministic teardown (IDisposable, closes its window) and switch every control test to 'using var host'; close the windows the HeadlessHarness tests show via try/finally. - Reset shared singletons in MainViewModelExtensionTests and dispose the MainViewModel via 'using var'; assert the close transition in ShellSmokeTests through ProjectService.CloseProject() rather than relying on Dispose(). - Scope the tests/CLAUDE.md singleton-reset guideline to shell tests that drive those singletons (the Beutl.E2ETests control/domain tests, which don't reference src/Beutl, neither need nor can perform the reset that CodeRabbit flagged). - Add the missing UTF-8 BOM to src/Beutl/Properties/AssemblyInfo.cs so the CI format check passes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Beutl.Editor.Components/FileBrowserTab/Services/MediaFileSearcher.cs`:
- Around line 143-145: Make MediaFileSearcher.Dispose() idempotent by preventing
later calls from touching an already-disposed CancellationTokenSource. Add an
early return at the start of Dispose() when _disposed is already true, and after
cancelling and disposing _searchCts, clear the field so subsequent Dispose()
calls do nothing. Use the existing _disposed flag and _searchCts field in
MediaFileSearcher to keep the cleanup safe across multiple invocations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e2d9be86-d0f4-4b56-81c7-fbcd7640a2f6
⛔ Files ignored due to path filters (2)
src/Beutl/runtimes/osx-arm64/native/libBeutlAVF.dylibis excluded by!**/*.dylibsrc/Beutl/runtimes/osx-x64/native/libBeutlAVF.dylibis excluded by!**/*.dylib
📒 Files selected for processing (21)
src/Beutl.Core/Services/INotificationService.cssrc/Beutl.Core/Services/Tutorials/TutorialService.cssrc/Beutl.Editor.Components/FileBrowserTab/Services/MediaFileSearcher.cssrc/Beutl/Properties/AssemblyInfo.cstests/Beutl.E2ETests/Controls/AlignmentEditorTests.cstests/Beutl.E2ETests/Controls/AutoCompleteStringEditorTests.cstests/Beutl.E2ETests/Controls/BooleanEditorTests.cstests/Beutl.E2ETests/Controls/ColorComponentsEditorTests.cstests/Beutl.E2ETests/Controls/ColorEditorTests.cstests/Beutl.E2ETests/Controls/EditorTestHost.cstests/Beutl.E2ETests/Controls/EnumEditorTests.cstests/Beutl.E2ETests/Controls/NumberEditorTests.cstests/Beutl.E2ETests/Controls/RationalEditorTests.cstests/Beutl.E2ETests/Controls/RelativeRectEditorTests.cstests/Beutl.E2ETests/Controls/StringEditorTests.cstests/Beutl.E2ETests/Controls/TimeSpanEditorTests.cstests/Beutl.E2ETests/Controls/VectorEditorTests.cstests/Beutl.E2ETests/HeadlessHarnessTests.cstests/Beutl.HeadlessUITests/MainViewModelExtensionTests.cstests/Beutl.HeadlessUITests/ShellSmokeTests.cstests/CLAUDE.md
✅ Files skipped from review due to trivial changes (2)
- src/Beutl/Properties/AssemblyInfo.cs
- tests/CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (17)
- tests/Beutl.E2ETests/Controls/EnumEditorTests.cs
- tests/Beutl.E2ETests/Controls/AlignmentEditorTests.cs
- tests/Beutl.E2ETests/Controls/RationalEditorTests.cs
- tests/Beutl.E2ETests/Controls/RelativeRectEditorTests.cs
- tests/Beutl.E2ETests/HeadlessHarnessTests.cs
- tests/Beutl.E2ETests/Controls/TimeSpanEditorTests.cs
- src/Beutl.Core/Services/INotificationService.cs
- tests/Beutl.E2ETests/Controls/AutoCompleteStringEditorTests.cs
- tests/Beutl.HeadlessUITests/MainViewModelExtensionTests.cs
- tests/Beutl.E2ETests/Controls/ColorEditorTests.cs
- tests/Beutl.E2ETests/Controls/BooleanEditorTests.cs
- tests/Beutl.HeadlessUITests/ShellSmokeTests.cs
- tests/Beutl.E2ETests/Controls/ColorComponentsEditorTests.cs
- tests/Beutl.E2ETests/Controls/NumberEditorTests.cs
- tests/Beutl.E2ETests/Controls/StringEditorTests.cs
- tests/Beutl.E2ETests/Controls/VectorEditorTests.cs
- tests/Beutl.E2ETests/Controls/EditorTestHost.cs
Dispose() set _disposed but still ran _searchCts?.Cancel() on a second call, throwing ObjectDisposedException on the already-disposed CancellationTokenSource. Return early when already disposed and null the field after disposing it, so Dispose() is safe to call multiple times.
Contributor
|
No TODO comments were found. |
Contributor
Minimum allowed line rate is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces the first end-to-end / headless-UI test infrastructure for Beutl, built on
Avalonia.Headless.NUnit— these suites run on headless CI (ubuntu) without xvfb or a GPU. Previously there were no E2E/UI tests at all.Three new projects under
tests/:Beutl.Testing.HeadlessBEUTL_HOMEtemp isolation, dispatcher/visual-tree helpers)Beutl.E2ETestssrc/Beutl): UI-less domain scenarios + headlessBeutl.Controlsproperty-editor testsBeutl.HeadlessUITestsProjectService/EditorService/MainViewModel/EditViewModel); referencessrc/BeutlCoverage: domain round-trips (Scene/Element/Project serialization, undo/redo via
HistoryManager+ theBeutl.Editoredit services, animation/keyframe, clipboard); 11 headless property editors driven by real simulated input (clicks/typing/wheel + value/confirm events); shell flows (create/open project, scene-editor tab activation, add-element/edit-property/save round-trip through the realEditViewModel, undo/redo via known editor commands, extension-list population,EditViewheadless inflation + frame capture); a GPU-gated preview-render test; and export-settings validation.Design change (opted into during review)
NotificationService.HandlerandTutorialService.Currenthadinternalsetters, so only the app assembly could register the handlers an external host needs. Their getters and interfaces are already public and designed for substitution, and a write-once??=guard provides the init-order safety theinternalmodifier did — so this makes the setters public (feat(core)), letting any host (the E2E harness, an alternative shell, automation) register them. This is non-breaking (the existingApp.RegisterServicescall site is unchanged) and removes one of the test-onlyInternalsVisibleTogrants. The property-editor handler stays internal, so two IVT grants remain (Beutl,Beutl.Extensibility).Convention change
tests/Beutl.HeadlessUITestsis the one test project that referencessrc/Beutl(the app exe), to drive the shell orchestration types that live there..claude/rules/csharp.mdis updated to record this exception. The GPL/MIT boundary is preserved:src/Beutl's reference toBeutl.FFmpegWorkerusesReferenceOutputAssembly="false", so the worker never enters the test compile closure.Harness notes worth knowing
Applicationuses Skia rendering (UseHeadlessDrawing = false) so real FluentAvalonia/Beutl templates (FontIcon glyphs, text) inflate headlessly.[AvaloniaTest]body, not in[SetUp]/[TearDown]— those run off the Avalonia UI thread, where mutating Avalonia/reactive state silently fails.TestAppreproducesApp.RegisterServices(handlers, registrars, primitive-extension load) minusApp.Initialize'sRunStartupTask(auth/update/last-project-restore network & disk I/O).Assert.Ignore+ logged reason) when no Vulkan/MoltenVK/SwiftShader device is present, mirroringBeutl.Graphics3DTests/VulkanTestEnvironment. The full FFmpeg-worker export E2E is intentionally not shipped (native FFmpeg hangs headless, the worker assembly is absent from test output, and worker IPC deadlocks the single-threaded headless dispatcher) — only non-gatedOutputViewModelvalidation is covered, documented inExportTests.cs.Verification
Full solution build + test pass locally (macOS/MoltenVK), with the existing suite unaffected:
Beutl.UnitTests3781,Beutl.E2ETests59,Beutl.HeadlessUITests24,Beutl.FFmpegIpc.Tests28,Beutl.Graphics3DTests5, MediaFoundation 55, AVFoundation 12, SourceGenerator 11 — 0 failed, 0 skipped.Reviewed across NUnit conventions, test honesty, the GPL/MIT boundary, and (for the public-API change) the "library-user flexibility" design priority; review feedback is addressed in the
address … review feedbackcommits.Summary by CodeRabbit