Skip to content

[Unit Tests] Add tests for TimeGate conditions and WindowBoundary types - #329

Open
DiamondDagger590 wants to merge 2 commits into
recodefrom
claude/magical-cray-0n5q9i
Open

[Unit Tests] Add tests for TimeGate conditions and WindowBoundary types#329
DiamondDagger590 wants to merge 2 commits into
recodefrom
claude/magical-cray-0n5q9i

Conversation

@DiamondDagger590

@DiamondDagger590 DiamondDagger590 commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add unit tests for TimeGateCondition and TimeGateChainConditionType (quest chain start condition system) — both previously at 0% coverage
  • Add unit tests for FixedWindowBoundaryType and RecurringWindowBoundaryType (availability window boundary parsing) — both previously at 0% coverage
  • All four classes are pure logic with no Bukkit dependencies, tested with plain JUnit + Mockito (mocked Section for YAML parsing)

Classes covered

Class Lines Key scenarios
TimeGateCondition 5 Boundary comparison (before/after/exact), timezone handling, edge cases
TimeGateChainConditionType 20 Valid parse, missing after, invalid date format, invalid timezone, default timezone
FixedWindowBoundaryType 15 Valid ISO-8601 parse, missing date, invalid format, warning log verification
RecurringWindowBoundaryType 17 Valid month-day+time parse, missing fields, invalid formats, warning log verification

Test plan


Generated by Claude Code

Summary by CodeRabbit

  • Tests
    • Added comprehensive coverage for fixed and recurring time-window boundaries.
    • Added validation for time-gated conditions, including date parsing, timezones, missing or invalid fields, and boundary timing.
    • Improved verification of metadata, key assignment, date/time preservation, and warning messages.

claude added 2 commits August 4, 2026 09:30
Cover four previously untested classes:
- TimeGateCondition: evaluate() boundary logic with timezone handling
- TimeGateChainConditionType: parse() with valid/invalid YAML sections
- FixedWindowBoundaryType: parse() for one-time date boundaries
- RecurringWindowBoundaryType: parse() for yearly recurring boundaries

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SYePW1DMHxkp5NZJ6F7bB8
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Claude finished @DiamondDagger590's task in 2m 47s —— View job


Review complete — 0 important, 1 nit

Scope: This PR adds unit tests only (src/test/java) for four previously 0%-covered pure-logic classes in the quest chain time-gate/availability system — no src/main changes. Per the routing table only the review-testing lens applies here; other lenses (security, architecture, concurrency, error-handling, performance, extensibility, gui-ux, server-owner) have no relevant surface to review since no production code changed.

I verified the new assertions against the actual source (TimeGateCondition, TimeGateChainConditionType, FixedWindowBoundaryType, RecurringWindowBoundaryType) — the boundary comparisons, exception messages, and logger warning text all match the real implementation. Mocking Section/Logger via Mockito is appropriate since these classes have no Bukkit runtime dependency.

Nits (1)
  • src/test/java/us/eunoians/mcrpg/quest/chain/condition/builtin/TimeGateConditionTest.java:23-25 — the whole test class is wrapped in a single @Nested ConditionTests group labeled "TimeGateCondition", which just duplicates the outer class name and adds no grouping benefit (unlike the sibling test files in this PR, which use @Nested to split multiple logical groups). Either flatten the tests to the top level or split into meaningful nested groups (e.g. EvaluateTests, GetKeyTests).

No blocking issues — coverage looks thorough (boundary edge cases, timezone handling, missing/invalid field parsing, and logger warning verification are all exercised).

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Added JUnit coverage for fixed and recurring window boundaries and for time-gate parsing and evaluation. The tests verify metadata, valid and invalid inputs, timezone behavior, boundary comparisons, empty results, and warning logs.

Changes

Quest chain time behavior

Layer / File(s) Summary
Window boundary parsing tests
src/test/java/us/eunoians/mcrpg/quest/chain/availability/builtin/WindowBoundaryTypeTest.java
Tests fixed and recurring window identifiers, valid parsing, invalid or missing fields, empty results, and warning logs.
Time-gate type parsing tests
src/test/java/us/eunoians/mcrpg/quest/chain/condition/builtin/TimeGateChainConditionTypeTest.java
Tests metadata, date and timezone parsing, defaults, validation errors, preserved time components, and returned keys.
Time-gate evaluation tests
src/test/java/us/eunoians/mcrpg/quest/chain/condition/builtin/TimeGateConditionTest.java
Tests boundary comparisons, one-second offsets, timezone conversion behavior, and condition keys.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the added unit tests for TimeGate conditions and WindowBoundary types.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/magical-cray-0n5q9i

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
src/test/java/us/eunoians/mcrpg/quest/chain/availability/builtin/WindowBoundaryTypeTest.java-70-72 (1)

70-72: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Guard the remaining direct casts with concrete-type assertions.

Add assertInstanceOf(..., result.get()) before the casts in WindowBoundaryTypeTest for the Fixed and Recurring tests that parse date/time components, and in TimeGateChainConditionTypeTest for the preserve-time-component test. This keeps the casts aligned with the casting guideline.

🤖 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/test/java/us/eunoians/mcrpg/quest/chain/availability/builtin/WindowBoundaryTypeTest.java`
around lines 70 - 72, Add assertInstanceOf guard assertions before the direct
casts to ensure type safety in tests. In WindowBoundaryTypeTest.java around
lines 70-72, add an assertInstanceOf check for WindowBoundary.Fixed before the
cast to fixed. In WindowBoundaryTypeTest.java around lines 166-169, add an
assertInstanceOf check for WindowBoundary.Recurring before the cast to
recurring. In TimeGateChainConditionTypeTest.java around lines 121-126, add an
assertInstanceOf check for the appropriate type before the cast in the
preserve-time-component test. Each assertInstanceOf should verify the type of
result.get() and be placed immediately before the corresponding cast to maintain
consistency with the casting guideline.

Source: Coding guidelines

🤖 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.

Other comments:
In
`@src/test/java/us/eunoians/mcrpg/quest/chain/availability/builtin/WindowBoundaryTypeTest.java`:
- Around line 70-72: Add assertInstanceOf guard assertions before the direct
casts to ensure type safety in tests. In WindowBoundaryTypeTest.java around
lines 70-72, add an assertInstanceOf check for WindowBoundary.Fixed before the
cast to fixed. In WindowBoundaryTypeTest.java around lines 166-169, add an
assertInstanceOf check for WindowBoundary.Recurring before the cast to
recurring. In TimeGateChainConditionTypeTest.java around lines 121-126, add an
assertInstanceOf check for the appropriate type before the cast in the
preserve-time-component test. Each assertInstanceOf should verify the type of
result.get() and be placed immediately before the corresponding cast to maintain
consistency with the casting guideline.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: 24a94095-56d8-4ca1-adfc-2c7d9eef694d

📥 Commits

Reviewing files that changed from the base of the PR and between cc181c0 and 85e7010.

📒 Files selected for processing (3)
  • src/test/java/us/eunoians/mcrpg/quest/chain/availability/builtin/WindowBoundaryTypeTest.java
  • src/test/java/us/eunoians/mcrpg/quest/chain/condition/builtin/TimeGateChainConditionTypeTest.java
  • src/test/java/us/eunoians/mcrpg/quest/chain/condition/builtin/TimeGateConditionTest.java

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