Test hardening: enablement rules with mocked view and geometry assertions

.github/workflows/ci-align.yml

@@ -0,0 +1,38 @@
+# Continuous integration for the align and distribute feature.
+#
+# Continuous integration means every change is built and tested automatically
+# when it is proposed, so problems show up at the moment of the pull request
+# instead of at merge time. This workflow builds the whole multi-module project
+# and runs the tests on every pull request and on pushes to my feature branches.
+#
+# The file is named ci-align.yml on purpose. A teammate already added a ci.yml on
+# another branch, so a separate name lets both pipelines run side by side and
+# avoids a conflict on the workflow file when the branches merge.
+name: CI Align and Distribute
+
+on:
+  pull_request:
+  push:
+    branches:
+      - "phongsakon/**"
+
+jobs:
+  build-and-test:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out the code
+        uses: actions/checkout@v4
+
+      # The project compiles at Java 1.8 source level but is built with JDK 11,
+      # so the runner uses Temurin 11 to match the local toolchain.
+      - name: Set up JDK 11
+        uses: actions/setup-java@v4
+        with:
+          distribution: temurin
+          java-version: "11"
+          cache: maven
+
+      # A clean build of every module. Tests run by default, which is the point
+      # of the pipeline, so there is no -DskipTests here.
+      - name: Build and test with Maven
+        run: mvn --batch-mode --no-transfer-progress clean install

jhotdraw-core/pom.xml

@@ -35,6 +35,41 @@
 			<version>6.8.21</version>
 			<scope>test</scope>
 		</dependency>
+		<!-- The project already ships TestNG, but the lab asks for JUnit 4 and
+		     notes that Swing and JUnit extensions work best with it. I add JUnit 4
+		     next to TestNG rather than replacing it, so the existing tests keep
+		     running and my new unit tests use JUnit 4. Surefire runs both in the
+		     same build. "test" scope means it is only on the classpath for tests
+		     and never ends up in the shipped jar. -->
+		<dependency>
+			<groupId>junit</groupId>
+			<artifactId>junit</artifactId>
+			<version>4.13.2</version>
+			<scope>test</scope>
+		</dependency>
+		<!-- Mockito lets the action-level tests stand in a fake DrawingView so the
+		     enabled-state rules can be checked without a live Swing editor. -->
+		<dependency>
+			<groupId>org.mockito</groupId>
+			<artifactId>mockito-core</artifactId>
+			<version>4.11.0</version>
+			<scope>test</scope>
+		</dependency>
+		<!-- JGiven turns the user story into runnable Given-When-Then scenarios on
+		     top of JUnit, and AssertJ gives the readable assertions the Then steps
+		     use. Both are test scoped, so they never reach the shipped jar. -->
+		<dependency>
+			<groupId>com.tngtech.jgiven</groupId>
+			<artifactId>jgiven-junit</artifactId>
+			<version>1.3.1</version>
+			<scope>test</scope>
+		</dependency>
+		<dependency>
+			<groupId>org.assertj</groupId>
+			<artifactId>assertj-core</artifactId>
+			<version>3.24.2</version>
+			<scope>test</scope>
+		</dependency>
 	 <dependency>
 	  <groupId>${project.groupId}</groupId>
 	  <artifactId>jhotdraw-actions</artifactId>

jhotdraw-core/src/main/java/org/jhotdraw/draw/action/AlignAction.java

@@ -112,6 +112,7 @@ protected void alignFigures(Collection<Figure> selectedFigures, Rectangle2D.Doub
                 f.willChange();
                 Rectangle2D.Double b = f.getBounds();
                 Point2D.Double d = alignment.delta(b, selectionBounds);
+                assert d != null : "an alignment must return a movement vector, never null";
                 AffineTransform tx = new AffineTransform();
                 tx.translate(d.x, d.y);
                 f.transform(tx);

jhotdraw-core/src/main/java/org/jhotdraw/draw/action/Distribution.java

@@ -81,11 +81,13 @@ public double size(Rectangle2D.Double r) {
      * @return the new leading coordinate per box, lined up with the input by index
      */
     public double[] newPositions(List<Rectangle2D.Double> boxes) {
+        assert boxes != null : "boxes must not be null";
         int n = boxes.size();
         double[] result = new double[n];
         for (int i = 0; i < n; i++) {
             result[i] = position(boxes.get(i));
         }
+        assert result.length == n : "one new leading coordinate per box";
         if (n < 3) {
             return result;
         }
@@ -122,6 +124,8 @@ public double[] newPositions(List<Rectangle2D.Double> boxes) {
             cursor += size(boxes.get(idx)) + gap;
         }
         result[lastIdx] = end;
+        assert result[firstIdx] == start && result[lastIdx] == end
+                : "the two outermost figures keep their place";
         return result;
     }
 }

jhotdraw-core/src/test/java/org/jhotdraw/draw/action/ActionEnablementTest.java

@@ -0,0 +1,65 @@
+/* @(#)ActionEnablementTest.java
+ *
+ * Copyright (c) the original authors of JHotDraw and all its contributors.
+ * You may not use, copy or modify this file, except in compliance with the
+ * accompanying license terms.
+ */
+package org.jhotdraw.draw.action;
+
+import org.jhotdraw.draw.DrawingEditor;
+import org.jhotdraw.draw.DrawingView;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Action-level tests for the enabled-state rules of the align and distribute
+ * actions. The Swing dependency is a mocked {@link DrawingView}, reached through
+ * a mocked {@link DrawingEditor}, so the rules can be checked without a live
+ * editor. This is the test double the TestLab asks for, and it covers the
+ * acceptance criteria the pure-geometry scenarios do not, the enable-at-two rule
+ * for align and the enable-at-three rule for distribute.
+ */
+public class ActionEnablementTest {
+
+    private DrawingEditor editor;
+    private DrawingView view;
+
+    @Before
+    public void setUp() {
+        editor = mock(DrawingEditor.class);
+        view = mock(DrawingView.class);
+        when(editor.getActiveView()).thenReturn(view);
+        when(view.isEnabled()).thenReturn(true);
+    }
+
+    @Test
+    public void alignIsDisabledWithOneFigureAndEnabledWithTwo() {
+        AlignAction.North align = new AlignAction.North(editor);
+
+        when(view.getSelectionCount()).thenReturn(1);
+        align.updateEnabledState();
+        assertFalse("align needs at least two figures", align.isEnabled());
+
+        when(view.getSelectionCount()).thenReturn(2);
+        align.updateEnabledState();
+        assertTrue("align is enabled at two figures", align.isEnabled());
+    }
+
+    @Test
+    public void distributeIsDisabledWithTwoFiguresAndEnabledWithThree() {
+        DistributeAction.Horizontal distribute = new DistributeAction.Horizontal(editor);
+
+        when(view.getSelectionCount()).thenReturn(2);
+        distribute.updateEnabledState();
+        assertFalse("distribute needs at least three figures", distribute.isEnabled());
+
+        when(view.getSelectionCount()).thenReturn(3);
+        distribute.updateEnabledState();
+        assertTrue("distribute is enabled at three figures", distribute.isEnabled());
+    }
+}

jhotdraw-core/src/test/java/org/jhotdraw/draw/action/AlignmentTest.java

@@ -0,0 +1,120 @@
+/*
+ * AlignmentTest.java
+ *
+ * New file added for the align and distribute feature.
+ * JHotDraw is distributed under the GNU LGPL v2.1.
+ */
+package org.jhotdraw.draw.action;
+
+import java.awt.geom.Point2D;
+import java.awt.geom.Rectangle2D;
+import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Unit tests for the {@link Alignment} geometry.
+ *
+ * <p>These test the pure part of the align feature, the movement each direction
+ * works out. This is exactly what the refactoring made possible. Before, the
+ * same arithmetic was buried inside Swing action subclasses and could only be
+ * checked by clicking the real buttons. Now it is a plain method from two
+ * rectangles to a vector, so a test just sets up the input, calls the method,
+ * and checks the number, with no editor and no mocks.</p>
+ *
+ * <p>Java notes to self. The {@code @Test} annotation marks a method JUnit runs.
+ * {@code assertEquals(expected, actual, EPS)} compares two doubles and the third
+ * argument is the tolerance, because comparing decimals for exact equality is
+ * unreliable. The fixture below is two boxes, a fixed selection and a smaller
+ * figure that moves, and each test comment states the expected delta.</p>
+ */
+public class AlignmentTest {
+
+    private static final double EPS = 1e-9;
+
+    /** Selection bounds: x=0, y=0, width=100, height=100. */
+    private Rectangle2D.Double selection() {
+        return new Rectangle2D.Double(0, 0, 100, 100);
+    }
+
+    /** A 20 by 20 figure sitting near the middle, at (40, 40). */
+    private Rectangle2D.Double figure() {
+        return new Rectangle2D.Double(40, 40, 20, 20);
+    }
+
+    // ---- Best case, each direction moves the figure where it should ----
+
+    @Test
+    public void northMovesTopEdgeToSelectionTop() {
+        // top of selection is y=0, figure top is y=40, so dy = -40 and dx = 0
+        Point2D.Double d = Alignment.NORTH.delta(figure(), selection());
+        assertEquals(0, d.x, EPS);
+        assertEquals(-40, d.y, EPS);
+    }
+
+    @Test
+    public void southMovesBottomEdgeToSelectionBottom() {
+        // selection bottom is y=100, figure bottom is 40+20=60, so dy = +40
+        Point2D.Double d = Alignment.SOUTH.delta(figure(), selection());
+        assertEquals(0, d.x, EPS);
+        assertEquals(40, d.y, EPS);
+    }
+
+    @Test
+    public void westMovesLeftEdgeToSelectionLeft() {
+        // selection left is x=0, figure left is x=40, so dx = -40
+        Point2D.Double d = Alignment.WEST.delta(figure(), selection());
+        assertEquals(-40, d.x, EPS);
+        assertEquals(0, d.y, EPS);
+    }
+
+    @Test
+    public void eastMovesRightEdgeToSelectionRight() {
+        // selection right is x=100, figure right is 40+20=60, so dx = +40
+        Point2D.Double d = Alignment.EAST.delta(figure(), selection());
+        assertEquals(40, d.x, EPS);
+        assertEquals(0, d.y, EPS);
+    }
+
+    @Test
+    public void verticalCentresOnHorizontalMidline() {
+        // selection mid y = 50, figure centre y = 40 + 10 = 50, already centred
+        Point2D.Double d = Alignment.VERTICAL.delta(figure(), selection());
+        assertEquals(0, d.x, EPS);
+        assertEquals(0, d.y, EPS);
+    }
+
+    @Test
+    public void horizontalCentresOnVerticalMidline() {
+        // selection mid x = 50, figure centre x = 50, already centred
+        Point2D.Double d = Alignment.HORIZONTAL.delta(figure(), selection());
+        assertEquals(0, d.x, EPS);
+        assertEquals(0, d.y, EPS);
+    }
+
+    // ---- Boundary cases ----
+
+    @Test
+    public void alreadyAlignedFigureDoesNotMove() {
+        // A figure already flush with the top should get a zero delta for NORTH.
+        Rectangle2D.Double flushTop = new Rectangle2D.Double(10, 0, 20, 20);
+        Point2D.Double d = Alignment.NORTH.delta(flushTop, selection());
+        assertEquals(0, d.x, EPS);
+        assertEquals(0, d.y, EPS);
+    }
+
+    @Test
+    public void figureLargerThanSelectionStillProducesConsistentDelta() {
+        // A figure wider than the selection still lines its left edge up for WEST.
+        Rectangle2D.Double wide = new Rectangle2D.Double(5, 40, 200, 20);
+        Point2D.Double d = Alignment.WEST.delta(wide, selection());
+        assertEquals(-5, d.x, EPS);
+        assertEquals(0, d.y, EPS);
+    }
+
+    @Test
+    public void deltaIsNeverNull() {
+        for (Alignment a : Alignment.values()) {
+            assertEquals(false, a.delta(figure(), selection()) == null);
+        }
+    }
+}