Skip to content

Conversation

@claude
Copy link

@claude claude bot commented Jan 4, 2026

Summary

  • Inspected ShiftLeftTest.kt for @TestWithoutNeovim annotation quality
  • Found 4 tests with missing description parameters
  • Added clear descriptions to all @TestWithoutNeovim annotations

Changes Made

Updated all 4 @TestWithoutNeovim annotations in ShiftLeftTest.kt:

  1. Line 17: Added "Caret positioning after shift with motion" to DIFFERENT reason
  2. Line 41: Added "Tests default startofline option behavior" to OPTION reason
  3. Line 65: Added "Tests nostartofline option behavior" to OPTION reason
  4. Line 91: Added "Tests nostartofline option with caret at end of line" to OPTION reason

This improves test documentation and makes it clearer why these tests are excluded from Neovim verification.

🤖 Generated with Claude Code

Added description parameters to all @TestWithoutNeovim annotations
in ShiftLeftTest to clarify why these tests are excluded from Neovim
verification:
- Test with DIFFERENT reason now explains caret positioning behavior
- Tests with OPTION reason now document which option behavior is tested

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
@claude
Copy link
Author

claude bot commented Jan 4, 2026

Code Review

Overall Assessment: Good improvement to test documentation. The added descriptions help clarify why tests are excluded from Neovim verification.

Issues Found:

  1. Line 17: Description "Caret positioning after shift with motion" is vague. The actual difference is that the caret should remain at column 31 (on 'l' in "land") but moves to the first non-blank character after shift. Consider: "IdeaVim moves caret to first non-blank after shift, Neovim preserves column"

  2. Lines 41, 65, 91: While technically accurate, the descriptions "Tests [no]startofline option behavior" don't explain WHY they skip Neovim. The startofline option determines whether line-oriented commands move to the first non-blank character. These tests verify IdeaVim's specific implementation. Consider more specific descriptions like:

    • Line 41: "Verifies IdeaVim startofline option (default) moves to first non-blank"
    • Line 65: "Verifies IdeaVim nostartofline preserves column position"
    • Line 91: "Verifies IdeaVim nostartofline behavior at line end"

Style: Descriptions should explain the specific behavioral difference or reason for skipping, not just restate what the test does. Compare with other tests in the codebase (e.g., "This test freezes", "bad replace term codes").

Test Coverage: ✓ All 4 @TestWithoutNeovim annotations now have descriptions
Build: Not verified, but changes are annotation-only


class ShiftLeftTest : VimTestCase() {
@TestWithoutNeovim(SkipNeovimReason.DIFFERENT)
@TestWithoutNeovim(SkipNeovimReason.DIFFERENT, "Caret positioning after shift with motion")
Copy link
Author

Choose a reason for hiding this comment

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

Description is too vague. The test shows that after <W (shift left with motion), IdeaVim positions the caret at the first non-blank character (column 0), but Neovim would keep it at the same visual column (around column 31). Consider: "IdeaVim moves caret to first non-blank after shift, Neovim preserves column"

}

@TestWithoutNeovim(SkipNeovimReason.OPTION)
@TestWithoutNeovim(SkipNeovimReason.OPTION, "Tests default startofline option behavior")
Copy link
Author

Choose a reason for hiding this comment

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

Description doesn't explain why Neovim is skipped. These tests verify IdeaVim's specific option implementation, not Neovim compatibility. Consider: "Verifies IdeaVim startofline option implementation"

}

@TestWithoutNeovim(SkipNeovimReason.OPTION)
@TestWithoutNeovim(SkipNeovimReason.OPTION, "Tests nostartofline option behavior")
Copy link
Author

Choose a reason for hiding this comment

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

Same as line 41 - focus on why it's excluded from Neovim testing rather than what the test does.

}

@TestWithoutNeovim(SkipNeovimReason.OPTION)
@TestWithoutNeovim(SkipNeovimReason.OPTION, "Tests nostartofline option with caret at end of line")
Copy link
Author

Choose a reason for hiding this comment

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

Same as line 41 - describe the IdeaVim-specific behavior being tested rather than restating the test name.

@AlexPl292 AlexPl292 closed this Jan 16, 2026
@AlexPl292 AlexPl292 deleted the tests-maintenance/add-neovim-skip-descriptions-shift-left branch January 16, 2026 11:49
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