Skip to content

Conversation

@pyramation
Copy link
Contributor

@pyramation pyramation commented Jul 20, 2025

Add test case for partial verification with toChange parameter

Summary

This PR implements and tests the toChange parameter functionality for module verification, allowing verification to stop at a specific point in the migration plan rather than verifying the entire module. The implementation includes:

  1. Core functionality: Added toChange parameter support in LaunchQLMigrate.verify() method to filter changes up to specified point
  2. Conditional logic: Implemented logic in LaunchQLProject.verify() to only apply toChange to the target module being verified, not its dependencies
  3. Comprehensive test: Created verify-partial.test.ts that demonstrates partial verification by breaking a verify script and showing that toChange limits verification scope
  4. Documentation: Added clear comments explaining why verification has different toChange semantics than deploy/revert operations

Key insight: Verification must check ALL dependencies for system integrity, while deploy/revert operations can safely apply toChange to all modules because they maintain consistency through ordered execution.

Review & Testing Checklist for Human

  • Verify conditional logic correctness: Review the toChange: extension === name ? toChange : undefined pattern in LaunchQLProject.verify() - ensure this only applies to the target module and not dependencies
  • Test end-to-end functionality: Manually deploy a module with multiple changes, break a verify script in the middle, then test launchql verify my-third --to-change=create_schema (should succeed) followed by launchql verify my-third (should fail)
  • Validate semantic differences: Confirm that different toChange behavior for verification vs deploy/revert makes operational sense and is properly documented
  • Check file modification robustness: Verify that the test's fs.writeFileSync approach properly cleans up and doesn't cause issues in CI environments
  • Test edge cases: Validate behavior with missing changes, circular dependencies, and complex module hierarchies

Recommended test plan: Deploy the my-third module from fixtures, manually edit create_table.sql verify script to break it, then run partial verification to confirm it stops at the specified point while full verification fails appropriately.


Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    CoreTestFixture["packages/core/test-utils/<br/>CoreDeployTestFixture.ts"]:::context
    LaunchQLProject["packages/core/src/core/class/<br/>launchql.ts"]:::major-edit
    LaunchQLMigrate["packages/core/src/migrate/<br/>client.ts"]:::major-edit
    VerifyPartialTest["packages/core/__tests__/projects/<br/>verify-partial.test.ts"]:::minor-edit
    VerifyScript["__fixtures__/sqitch/simple-w-tags/<br/>packages/my-third/verify/create_table.sql"]:::context

    
    CoreTestFixture -->|"calls verifyModule()"| LaunchQLProject
    LaunchQLProject -->|"conditional toChange logic<br/>extension === name"| LaunchQLMigrate
    LaunchQLMigrate -->|"filters changes<br/>up to toChange"| LaunchQLMigrate
    VerifyPartialTest -->|"modifies verify script<br/>to test failure scenarios"| VerifyScript
    VerifyPartialTest -->|"calls fixture.verifyModule()<br/>with toChange param"| CoreTestFixture

    subgraph Legend
        L1[Major Edit]:::major-edit
        L2[Minor Edit]:::minor-edit  
        L3[Context/No Edit]:::context
    end

    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB
    classDef context fill:#FFFFFF
Loading

Notes

  • The conditional toChange logic is the highest risk area - it determines when partial verification applies and could easily break if the logic is incorrect
  • The test uses file system modification which could be brittle if fixture paths change or cleanup fails
  • Deploy/revert operations can safely pass toChange to all modules because they are idempotent and maintain consistency, unlike verification which must check all dependencies
  • The findIndex and slice operations in the filtering logic need validation for edge cases like missing changes
  • Session requested by Dan Lynch (@pyramation)
  • Link to Devin run: https://app.devin.ai/sessions/c7b2798f4c5d439db9ec89800c9a1155

@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

- Implement toChange parameter support in LaunchQLMigrate.verify method
- Add logic to filter changes up to specified toChange point
- Fix recursive verification to only apply toChange to target module
- Add comprehensive test case demonstrating partial verification functionality
- Test verifies that toChange limits verification scope and throws on failure

Co-Authored-By: Dan Lynch <[email protected]>
- Add clear comments explaining why toChange only applies to target module
- Clarify that verification differs from deploy/revert semantics
- Dependencies must be fully verified to ensure system integrity
- Addresses user feedback about confusing conditional logic

Co-Authored-By: Dan Lynch <[email protected]>
- Document why deploy/revert can safely pass toChange to all modules
- Explain that deploy operations are idempotent and safe to stop early
- Clarify that revert works backwards maintaining consistency
- Contrast with verification which must check all dependencies for integrity
- Addresses user feedback about explaining the semantic differences

Co-Authored-By: Dan Lynch <[email protected]>
@pyramation pyramation changed the title Add test case for partial verification with toChange parameter Add test case for partial verification with toChange parameter (test-case/verify) Jul 21, 2025
@pyramation pyramation closed this Jul 21, 2025
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