Draw a baseline bar when only the duration changed

[Natalie-the-technician]

This replaces #2846, which I am closing.

You were right about the colours there. The end date is what a schedule is
judged by, so a task that gets longer or shorter while still meeting its
deadline should not be flagged as a delay. That half is dropped. The colours
in this PR are exactly the ones on master: later when the task now ends
later than planned, earlier when it ends sooner, decided by the end date
alone.

What is left is a separate bug.

The bug. When a task keeps its end date but its duration changed,
renderBaseline returns early and no baseline bar is drawn at all. The user
sees nothing -- not a neutral bar, nothing -- even though the task no longer
matches its baseline.

Measured on master 92a91e9: a task with a baseline of 10 Aug + 5 days
(ending 15 Aug) that now runs 5 Aug + 10 days (also ending 15 Aug) draws
0 baseline bars where 1 is expected. The positive control -- the same
harness and the same task, with only the end date shifted -- draws 1 and is
green, so the zero is the finding and not a broken test.

This is independent of how the colours are chosen.

The change. +21/-10 lines of code in one file,
GanttChartSceneBuilder.java, in two places: the "should there be a bar, and
which styles" decision moves out of renderBaseline into a static
getBaselineStyles(...), and the guard becomes

// was: if (endDate.equals(t.getEnd().getTime())) return;
if (endComparison == 0 && currentDuration == baselineDuration) {
  return null;   // nothing changed at all -> still no bar
}

so a bar is drawn whenever anything changed, not only when the end date
moved. The styles are then added from the end date comparison exactly as
before. Durations are compared as the whole units the baseline stored, which
is the number GanttPreviousState.createTasks() took from the task in the
first place.

One consequence worth naming. A bar for a task that still ends on the
planned day carries neither earlier nor later, so StyledPainterImpl
falls through to getPreviousTaskColor() -- the existing "Task remains on
schedule" preference. On master that preference cannot reach a baseline bar
at all: there is a single place that creates a previousStateTask shape and
it always adds one of the two deviation styles. This patch is what makes that
preference do something. That is read from the code; I did not check the
colour on screen.

Beyond this PR. My fork does this differently, since you have looked there
already: the single band became three. One compares end dates anchored at the
planned start, one compares durations anchored at today's start, and one compares
recorded hours against the original estimate -- that last one needs the effort
tracking and would do nothing here. It is a design change rather than a bug fix,
so this PR leaves master's single band as it is.

Tests. BaselineBarCountTest.kt (2 tests) drives the production
GanttChartSceneBuilder through a hand-written InputApi, calls render()
and counts the shapes on the finished canvas whose style is
previousStateTask. That is the test that shows 0-vs-1 on master.
BaselineStylesTest.kt (8 tests) pins the style decision, six of them nailing
down today's colour behaviour so it cannot drift. Full run on this branch:
BUILD SUCCESSFUL, 408 tests, 0 failures, 0 errors.

Negative control: taking just && currentDuration == baselineDuration back
out of the guard and changing nothing else turns 3 of the 10 new tests red,
and they are the 3 that were predicted -- the bar count test, and the two
"grew / shrank but still ends on the planned day" style tests. The other 7
stay green, which is right: they do not touch this case. Putting the
condition back makes all 10 green again.

Why a new PR rather than a push to #2846. Of the 9 tests on that branch,
not one tests this bug on its own: 4 hold under either semantics, 3 are purely
about the colours, and 2 mix the two in a single assertion. There was nothing
in it you could read as "this is the missing-bar fix", so a fresh small PR
seemed cleaner than force-pushing over a review you had already answered.

[dbarashev]

There is an existing test implementation in TaskRendererImplTest. I suggest extracting these test implementations into their own file and reusing in different tests, if that makes sense.

[Natalie-the-technician]

Done, with one deviation I should flag up front.

The two implementations were in different Gradle modules. TaskRendererImplTest
is in ganttproject-tester; my test was in ganttproject/src/test.
ganttproject-tester depends on the main source set of :ganttproject, so
nothing in ganttproject/src/test is on its compile classpath — I checked with a
throwaway class: cannot find symbol from ganttproject/src/test, compiles fine
once the same class is moved to ganttproject/src/main. There is no
java-test-fixtures setup anywhere in the build, and the project's current answer
to this problem is a second copy: TestSetupHelper.java exists in both modules
and the two copies have already drifted apart.

Rather than introduce a new sharing mechanism, I moved BaselineBarCountTest into
ganttproject-tester, where the other chart tests live, and put the shared
implementation in
ganttproject-tester/test/net/sourceforge/ganttproject/chart/gantt/TestSceneTask.kt
— same directory and same naming as TestPainter, TestOffsetBuilder and
TestTextLengthCalculator. BaselineStylesTest stays in ganttproject/src/test
because it calls the package-private getBaselineStyles. Happy to do it
differently if you would rather have test fixtures, or keep the test where it was.

What could and could not be shared. Eight of the fourteen members were
identical in both implementations — isCritical, isProjectTask,
hasNestedTasks, shape, notes, completionPercentage, isMilestone,
getProperty — and those now exist once, in an abstract base. The other six could
not be shared, because the two tests answer them from different places:

• getRowId and expand: TaskRendererImplTest reads them through to a live
  Task, and it has to — testVerticalPartitioningWithCollapsedTasks collapses
  the task after wrapping it. Replacing the read-through with a constant makes
  that test fail (expected:<2> but was:<4>), so folding it into my constant
  expand would have quietly stopped it testing collapsing at all.
• activities, duration, end, color: BaselineBarCountTest needs real
  values here, and it deliberately has no task manager and no chart model to take
  them from. Making activities return an empty list fails both of its tests;
  falsifying duration fails one.

So they are two small subclasses in the one file, RealTaskSceneTask (two lines of
body) and SingleActivitySceneTask (six).

One deliberate behaviour change. TaskRendererImplTest's implementation
returned null from getColor, getEnd, getActivities and getDuration, all
four declared @NotNull. I checked whether anything actually reads them: making
all four throw leaves all four tests green, so they were dead. The shared base now
throws with the name of the member instead, so a chart test that starts to depend
on one of them fails loudly rather than reading a null out of a non-null property.

Evidence that no check got weaker. I extracted every assert* call with its
arguments and its message from the three test files before and after the change —
65 of them — and the two lists are identical; the same 14 tests run. Both modules
are fully green: :ganttproject-tester:test 287 tests, :ganttproject:test 100
tests. And the tests still detect what they exist for: dropping the duration
comparison from getBaselineStyles turns exactly three of them red — the bar-count
test for the equal-end-date case, and the two style tests for a task that grew or
shrank without moving its end date.

Net effect: 89 insertions, 116 deletions.