Skip to content

Conversation

@theosanderson
Copy link
Member

@theosanderson theosanderson commented Jan 16, 2026

This is an alternative to #5830 which tries to preserve a bit of the functionality. No super strong feelings @chaoran-chen if you prefer yours, sorry for the delay.

[clauded]
This PR cleans up unused code in the integration test page objects by:

  • Removing methods that are duplicated or unnecessary
  • Adding test coverage for methods that were unused but useful

The goal is to ensure all page object methods have test coverage, making the codebase easier to maintain and preventing dead code accumulation.

Changes

Removed (duplicated/unnecessary[edit:theo also / just did not get round to adding back, for revision cases)

Method Reason
SingleSequenceSubmissionPage.fillField() Duplicates EditPage.fillField()
SequenceDetailPage.getPage() Unnecessary getter that just returns this.page
RevisionPage.clickReviseSequenceLink() Duplicates SearchPage.reviseSequence()
RevisionPage.uploadSegmentFile() Unused, no test coverage
RevisionPage.discardSegmentFile() Unused, no test coverage
RevisionPage.discardSequenceFile() Unused, no test coverage
RevisionPage.discardMetadataFile() Unused, no test coverage
RevisionPage.clickConfirm() Unused, no test coverage
RevisionPage.reviseSequence() Unused convenience wrapper
RevisionPage.reviseMultiSegmentSequence() Unused convenience wrapper

Added test coverage

Method Test file
SearchPage.getSequencePreviewModal() sequence-preview-url.dependent.spec.ts
SearchPage.getHalfScreenPreview() sequence-preview-url.dependent.spec.ts
CliPage.authStatus() auth.spec.ts
SequenceDetailPage.selectAlignedTab() sequence-detail-page.dependent.spec.ts
GroupPage.editAddressLine2() management.spec.ts
ReviewPage.navigateToReviewPage() review-page.spec.ts
RevisionPage.downloadTsvTemplate() revise-sequence.spec.ts
RevisionPage.downloadXlsxTemplate() revise-sequence.spec.ts
CliPage.setDefaultOrganism() organism-group.spec.ts (new)
CliPage.clearDefaultOrganism() organism-group.spec.ts (new)
CliPage.setDefaultGroup() organism-group.spec.ts (new)
CliPage.clearDefaultGroup() organism-group.spec.ts (new)
CliPage.getAvailableOrganisms() organism-group.spec.ts (new)
CliPage.getAvailableGroups() organism-group.spec.ts (new)

Bug fix

  • Fixed SequenceDetailPage.alignedTab getter to use .first() to handle pages with multiple aligned tabs (aligned nucleotide and aligned amino acid sequences)

Test plan

  • All modified tests pass against main.loculus.org
  • New organism-group.spec.ts CLI tests pass
  • New revision template download test passes
  • ESLint and Prettier pass

🤖 Generated with Claude Code

🚀 Preview: Add preview label to enable

@claude claude bot added the e2e E2E test related label Jan 16, 2026
@theosanderson theosanderson changed the base branch from main to remove-unused January 16, 2026 21:20
@theosanderson theosanderson changed the base branch from remove-unused to main January 16, 2026 21:21
@theosanderson theosanderson changed the base branch from main to remove-unused January 16, 2026 21:21
@theosanderson theosanderson changed the base branch from remove-unused to main January 16, 2026 21:22
@theosanderson theosanderson marked this pull request as ready for review January 16, 2026 21:24
@claude
Copy link
Contributor

claude bot commented Jan 16, 2026

Claude finished @theosanderson's task in 3m 32s —— View job


Code Review Complete ✓

This is a solid cleanup PR that removes genuinely unused code and adds meaningful test coverage. The changes improve code maintainability by ensuring all page object methods are actually tested.

Summary

Strengths:

  • All removed methods are confirmed unused in the codebase
  • New test coverage is well-structured and comprehensive
  • Bug fix for alignedTab selector correctly handles multiple aligned tabs
  • Code organization is improved by removing duplication

Areas for improvement:

  • Some test assertions could be more specific (see inline comments)
  • Consider validating template download content/format, not just filenames

Detailed Findings

Removed methods verification:

  • SingleSequenceSubmissionPage.fillField() - unused, duplicates EditPage.fillField()
  • SequenceDetailPage.getPage() - unused getter
  • ✅ All RevisionPage methods - confirmed unused (similar functionality in EditPage where needed)

Test coverage additions:

  • organism-group.spec.ts - comprehensive CLI testing for organism/group commands
  • ✅ Template download tests - verify functionality works
  • ✅ Review page navigation test - ensures UI path works
  • ✅ Sequence detail page aligned tab test - covers the bug fix

Bug fix (sequence-detail.page.ts:16):
The addition of .first() to the alignedTab getter correctly handles pages with multiple aligned tabs (e.g., "aligned nucleotide" and "aligned amino acid sequences"). This prevents selector ambiguity.

Test Quality

The new tests follow good patterns:

  • Proper setup and teardown
  • Clear test descriptions
  • Appropriate timeouts
  • Good use of assertions

See inline comments for specific suggestions on improving assertion specificity.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2e E2E test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants