Skip to content

refactor: enforce UI-driven interaction hierarchy in integration tests#769

Merged
DavidGershony merged 2 commits intomainfrom
fix/integration-test-ui-driven-refactor
Apr 20, 2026
Merged

refactor: enforce UI-driven interaction hierarchy in integration tests#769
DavidGershony merged 2 commits intomainfrom
fix/integration-test-ui-driven-refactor

Conversation

@dangershony
Copy link
Copy Markdown
Member

Summary

  • Removed direct new InvestPageViewModel(...) construction from 3 test files (OneClickInvestLightningTest, OneClickInvestOnChainTest, InvestModalsViewFixesTest) — tests now navigate through the UI (Find Projects → OpenProjectDetail → OpenInvestPage) to obtain the ViewModel
  • Added // DIRECT SDK CALL: and // DIAGNOSTIC SDK CALL: comments to 4 test files where SDK bypass is justified, documenting why no ViewModel exposes the needed data
  • Updated TESTING_GUIDELINES.md with interaction hierarchy rules (UI clicks > VM method calls > direct SDK calls) and expanded the test inventory from 6 to 14 entries
  • Created 4 missing markdown docs for OneClickInvestLightningTest, OneClickInvestOnChainTest, InvestModalsViewFixesTest, and FindProjectsPanelTests

All 19 integration tests pass individually.

- Remove direct InvestPageViewModel construction (new) from 3 tests;
  use UI navigation (OpenProjectDetail → OpenInvestPage) instead
- Add DIRECT SDK CALL / DIAGNOSTIC SDK CALL comments to justified
  bypass locations in 4 test files
- Update TESTING_GUIDELINES.md with interaction hierarchy rules
- Add missing markdown docs for 4 tests
@dangershony
Copy link
Copy Markdown
Member Author

Detailed Summary

Problem

Several integration tests were bypassing the UI layer by constructing ViewModels directly with new InvestPageViewModel(...), pulling dependencies out of the DI container manually. This made tests fragile (they break when constructor signatures change) and didn't validate the actual navigation paths users take. Additionally, some tests made direct SDK service calls without documenting why the UI/ViewModel layer couldn't be used instead.

What Changed

1. Eliminated direct ViewModel construction (3 files)

These tests previously resolved IServiceProvider from the test harness, pulled individual services, and called new InvestPageViewModel(...):

  • OneClickInvestLightningTest.cs (both LightningInvoiceFlow_FullStateMachine and PortfolioDeduplication_AfterInvestment)
  • OneClickInvestOnChainTest.cs (both OnChainInvoiceFlow_FullStateMachine and OnChainInvoiceFlow_TabSwitchRaceCondition)
  • InvestModalsViewFixesTest.cs (InvestModals_FixesAreWired)

All now use the UI navigation path: GetFindProjectsViewModel()OpenProjectDetail(project)OpenInvestPage() → read findVm.InvestPageViewModel. This matches how the real app creates the InvestPageViewModel via the Func<ProjectItemViewModel, InvestPageViewModel> factory registered in CompositionRoot.cs.

2. Documented justified SDK bypass locations (4 files)

Added standardized comments explaining why direct SDK calls are necessary:

  • CreateProjectTest.cs (~line 444) — // DIRECT SDK CALL: fetching the full ProjectDto to validate stage data that no ViewModel exposes
  • FindProjectsInvoiceFlowTest.cs (~line 236) — // DIRECT SDK CALL: fetching project stats to verify investor count, not available through any VM
  • MultiFundClaimAndRecoverTest.cs// DIAGNOSTIC SDK CALL: on LogRecoveryBuildDiagnostics helper used in retry loops for troubleshooting flaky recovery builds
  • MultiInvestClaimAndRecoverTest.cs// DIAGNOSTIC SDK CALL: on LogUnfundedReleaseBuildDiagnostics helper, same diagnostic purpose

3. Updated TESTING_GUIDELINES.md

  • Added Interaction Hierarchy section defining the preference order: UI-level helpers (TestHelpers.Click*) > ViewModel method calls > direct SDK service calls
  • Added rules against constructing ViewModels with new in tests
  • Added guidance on when SDK calls are acceptable and how to document them
  • Expanded the test inventory from 6 to 14 entries covering all current test files

4. Created 4 missing markdown docs

  • OneClickInvestLightningTest.md
  • OneClickInvestOnChainTest.md
  • InvestModalsViewFixesTest.md
  • FindProjectsPanelTests.md

Verification

All 19 integration tests were run individually and pass:

  • 2 smoke tests
  • 5 refactored tests (the ones that changed)
  • 3 FindProjectsPanelTests
  • 9 infrastructure tests (SendToSelf, WalletImport, CreateProject, FindProjectsInvoice, FundAndRecover, InvestAndRecover, InvestmentCancellation, MultiFundClaimAndRecover, MultiInvestClaimAndRecover)


## Purpose

Fast smoke test verifying recent fixes to the InvestPageViewModel and InvestModalsView. Tests that error bindings, immediate UX feedback, tab-driven labels, and defensive error labeling all work correctly without needing a funded wallet or testnet faucet.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@DavidGershony I wonder do we need this test? it feels more like a unit test then integration what do you think?

The elements being tests are probably already covered in a full integration test.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We still had the issue that nothing worked at some point.

@DavidGershony DavidGershony self-requested a review April 20, 2026 10:20
@DavidGershony DavidGershony merged commit a6874e6 into main Apr 20, 2026
3 checks passed
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.

2 participants