Skip to content

Fix Edge Case in Anchor Validation#1822

Merged
dandelany merged 3 commits into
developfrom
fix/anchor-validation
May 8, 2026
Merged

Fix Edge Case in Anchor Validation#1822
dandelany merged 3 commits into
developfrom
fix/anchor-validation

Conversation

@Mythicaeda
Copy link
Copy Markdown
Contributor

@Mythicaeda Mythicaeda commented Apr 30, 2026

Description

Previously, the "activity is before plan start" validation check would skip activities that did not have a negative start offset. This would cause it to fail to mark activities as "out of bounds" in the case where it had a positive offset to its out-of-bounds parent anchor, but not enough of a positive offset to put it back inside plan bounds. The logic has been fixed to check all start time anchors, not just negative start time anchors.

Verification

A new test was added to AnchorTests that goes over the formerly-buggy edge case: NetNegativePlanStartStatus.indirectlyInvalidChild

Additionally, the AnchorTests have been refactored, with a focus on making them more direct and better commented. See the second commit for a more thorough list of changes.

@Mythicaeda Mythicaeda self-assigned this Apr 30, 2026
@Mythicaeda Mythicaeda requested a review from a team as a code owner April 30, 2026 15:17
@Mythicaeda Mythicaeda added test Adding missing tests, correcting existing tests, or test infrastructure database Anything related to the database labels Apr 30, 2026
@Mythicaeda Mythicaeda force-pushed the fix/anchor-validation branch from cca1ac3 to df07bc1 Compare April 30, 2026 15:19
@Mythicaeda Mythicaeda force-pushed the fix/anchor-validation branch from df07bc1 to d62a91b Compare April 30, 2026 15:28
@Mythicaeda Mythicaeda added refactor A code change that neither fixes a bug nor adds a feature fix A bug fix labels May 1, 2026
Mythicaeda added 2 commits May 6, 2026 16:36
Before, when checking whether an activity had a negative offset relative to plan_start, it would skip activities that had a positive start offset, even if their total offset was negative (ie, due to the activity it was anchored to having a large negative offset).
Tests now:
 - are better commented
 - use JUnit's "assertThrows" instead of try-catch
 - are more direct
 - have shared setup code pulled out into the BeforeEach
Copy link
Copy Markdown
Contributor

@AaronPlave AaronPlave left a comment

Choose a reason for hiding this comment

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

@dandelany and I tested this locally and added one e2e test.

@dandelany
Copy link
Copy Markdown
Collaborator

Merging so i can get tests running on #1825 - thanks!

@dandelany dandelany merged commit 27c5818 into develop May 8, 2026
17 checks passed
@dandelany dandelany deleted the fix/anchor-validation branch May 8, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

database Anything related to the database fix A bug fix refactor A code change that neither fixes a bug nor adds a feature test Adding missing tests, correcting existing tests, or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Anchored directive validation failing to catch plan bound violations in some cases

3 participants