From 5312252fee162e6bc9bf5ac355930e16f1776c53 Mon Sep 17 00:00:00 2001 From: kiramt Date: Thu, 11 May 2017 14:30:49 +0100 Subject: [PATCH] JAL-2491 Unit tests and updates --- src/jalview/gui/AlignFrame.java | 5 +- src/jalview/gui/AlignmentPanel.java | 27 +- src/jalview/viewmodel/OverviewDimensions.java | 5 +- src/jalview/viewmodel/ViewportRanges.java | 89 ++++- test/jalview/viewmodel/OverviewDimensionsTest.java | 6 +- test/jalview/viewmodel/ViewportRangesTest.java | 357 +++++++++++++++++++- 6 files changed, 437 insertions(+), 52 deletions(-) diff --git a/src/jalview/gui/AlignFrame.java b/src/jalview/gui/AlignFrame.java index bee5e5b..8d26d4f 100644 --- a/src/jalview/gui/AlignFrame.java +++ b/src/jalview/gui/AlignFrame.java @@ -685,8 +685,7 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener, } else { - vpRanges.setStartSeq(2 * vpRanges.getStartSeq() - - vpRanges.getEndSeq()); + vpRanges.pageUp(); } break; case KeyEvent.VK_PAGE_DOWN: @@ -696,7 +695,7 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener, } else { - vpRanges.setStartSeq(vpRanges.getEndSeq()); + vpRanges.pageDown(); } break; } diff --git a/src/jalview/gui/AlignmentPanel.java b/src/jalview/gui/AlignmentPanel.java index e545b8f..6425088 100644 --- a/src/jalview/gui/AlignmentPanel.java +++ b/src/jalview/gui/AlignmentPanel.java @@ -758,21 +758,20 @@ public class AlignmentPanel extends GAlignmentPanel implements { vpRanges.setViewportStartAndWidth(offy * rowSize, rowSize); } - else + } + else + { + // This is only called if file loaded is a jar file that + // was wrapped when saved and user has wrap alignment true + // as preference setting + SwingUtilities.invokeLater(new Runnable() { - // This is only called if file loaded is a jar file that - // was wrapped when saved and user has wrap alignment true - // as preference setting - SwingUtilities.invokeLater(new Runnable() - { - @Override - public void run() - { - setScrollValues(vpRanges.getStartRes(), - vpRanges.getStartSeq()); - } - }); - } + @Override + public void run() + { + setScrollValues(vpRanges.getStartRes(), vpRanges.getStartSeq()); + } + }); } repaint(); } diff --git a/src/jalview/viewmodel/OverviewDimensions.java b/src/jalview/viewmodel/OverviewDimensions.java index 43680b5..56060ab 100644 --- a/src/jalview/viewmodel/OverviewDimensions.java +++ b/src/jalview/viewmodel/OverviewDimensions.java @@ -156,7 +156,7 @@ public class OverviewDimensions int xAsRes = Math.round((float) x * alwidth / width); // get viewport width in residues - int vpwidth = ranges.getEndRes() - ranges.getStartRes() + 1; + int vpwidth = ranges.getViewportWidth(); // get where x should be when accounting for hidden cols // if x is in a hidden col region, shift to left - but we still need @@ -190,8 +190,7 @@ public class OverviewDimensions int yAsSeq = Math.round((float) y * alheight / sequencesHeight); // get viewport height in sequences - // add 1 because height includes both endSeq and startSeq - int vpheight = ranges.getEndSeq() - ranges.getStartSeq() + 1; + int vpheight = ranges.getViewportHeight(); // get where y should be when accounting for hidden rows // if y is in a hidden row region, shift up - but we still need absolute diff --git a/src/jalview/viewmodel/ViewportRanges.java b/src/jalview/viewmodel/ViewportRanges.java index ebacab1..d77baf5 100644 --- a/src/jalview/viewmodel/ViewportRanges.java +++ b/src/jalview/viewmodel/ViewportRanges.java @@ -81,6 +81,7 @@ public class ViewportRanges extends ViewportProperties /** * Set first residue visible in the viewport, and retain the current width. + * Fires a property change event. * * @param res * residue position @@ -103,7 +104,7 @@ public class ViewportRanges extends ViewportProperties */ public void setStartEndRes(int start, int end) { - int oldres = this.startRes; + int oldstartres = this.startRes; if (start > al.getWidth() - 1) { startRes = al.getWidth() - 1; @@ -117,6 +118,7 @@ public class ViewportRanges extends ViewportProperties startRes = start; } + int oldendres = this.endRes; if (end < 0) { endRes = 0; @@ -126,11 +128,17 @@ public class ViewportRanges extends ViewportProperties endRes = end; } - changeSupport.firePropertyChange("startres", oldres, start); + changeSupport.firePropertyChange("startres", oldstartres, start); + if (oldstartres == start) + { + // event won't be fired if start positions are same + // fire an event for the end positions in case they changed + changeSupport.firePropertyChange("endres", oldendres, end); + } } /** - * Set last residue visible in the viewport + * Set last residue visible in the viewport. Fires a property change event. * * @param res * residue position @@ -142,7 +150,8 @@ public class ViewportRanges extends ViewportProperties } /** - * Set the first sequence visible in the viewport + * Set the first sequence visible in the viewport. Fires a property change + * event. * * @param seq * sequence position @@ -165,7 +174,7 @@ public class ViewportRanges extends ViewportProperties */ public void setStartEndSeq(int start, int end) { - int oldseq = this.startSeq; + int oldstartseq = this.startSeq; if (start > al.getHeight() - 1) { startSeq = al.getHeight() - 1; @@ -179,6 +188,7 @@ public class ViewportRanges extends ViewportProperties startSeq = start; } + int oldendseq = this.endSeq; if (end >= al.getHeight()) { endSeq = al.getHeight() - 1; @@ -192,11 +202,18 @@ public class ViewportRanges extends ViewportProperties endSeq = end; } - changeSupport.firePropertyChange("startseq", oldseq, start); + changeSupport.firePropertyChange("startseq", oldstartseq, start); + if (oldstartseq == start) + { + // event won't be fired if start positions are the same + // fire in case the end positions changed + changeSupport.firePropertyChange("endseq", oldendseq, end); + } } /** - * Set the last sequence visible in the viewport + * Set the last sequence visible in the viewport. Fires a property change + * event. * * @param seq * sequence position @@ -242,7 +259,7 @@ public class ViewportRanges extends ViewportProperties /** * Set viewport width in residues, without changing startRes. Use in * preference to calculating endRes from the width, to avoid out by one - * errors! + * errors! Fires a property change event. * * @param w * width in residues @@ -255,7 +272,7 @@ public class ViewportRanges extends ViewportProperties /** * Set viewport height in residues, without changing startSeq. Use in * preference to calculating endSeq from the height, to avoid out by one - * errors! + * errors! Fires a property change event. * * @param h * height in sequences @@ -267,7 +284,8 @@ public class ViewportRanges extends ViewportProperties /** * Set viewport horizontal start position and width. Use in preference to - * calculating endRes from the width, to avoid out by one errors! + * calculating endRes from the width, to avoid out by one errors! Fires a + * property change event. * * @param start * start residue @@ -276,12 +294,22 @@ public class ViewportRanges extends ViewportProperties */ public void setViewportStartAndWidth(int start, int w) { - setStartEndRes(start, start + w - 1); + int vpstart = start; + if (vpstart < 0) + { + vpstart = 0; + } + else if (vpstart + w - 1 > al.getWidth() - 1) + { + vpstart = al.getWidth() - 1; + } + setStartEndRes(vpstart, vpstart + w - 1); } /** * Set viewport vertical start position and height. Use in preference to - * calculating endSeq from the height, to avoid out by one errors! + * calculating endSeq from the height, to avoid out by one errors! Fires a + * property change event. * * @param start * start sequence @@ -290,7 +318,16 @@ public class ViewportRanges extends ViewportProperties */ public void setViewportStartAndHeight(int start, int h) { - setStartEndSeq(start, start + h - 1); + int vpstart = start; + if (vpstart < 0) + { + vpstart = 0; + } + else if (vpstart + h - 1 > al.getHeight() - 1) + { + vpstart = al.getHeight() - h; + } + setStartEndSeq(vpstart, vpstart + h - 1); } /** @@ -314,7 +351,7 @@ public class ViewportRanges extends ViewportProperties } /** - * Scroll the viewport range vertically + * Scroll the viewport range vertically. Fires a property change event. * * @param up * true if scrolling up, false if down @@ -345,7 +382,7 @@ public class ViewportRanges extends ViewportProperties } /** - * Scroll the viewport range horizontally + * Scroll the viewport range horizontally. Fires a property change event. * * @param right * true if scrolling right, false if left @@ -377,7 +414,8 @@ public class ViewportRanges extends ViewportProperties } /** - * Scroll a wrapped alignment so that the specified residue is visible + * Scroll a wrapped alignment so that the specified residue is visible. Fires + * a property change event. * * @param res * residue position to scroll to @@ -391,7 +429,7 @@ public class ViewportRanges extends ViewportProperties } /** - * Scroll so that (x,y) is visible + * Scroll so that (x,y) is visible. Fires a property change event. * * @param x * x position in alignment @@ -427,5 +465,20 @@ public class ViewportRanges extends ViewportProperties } } } - + + /** + * Adjust sequence position for page up. Fires a property change event. + */ + public void pageUp() + { + setViewportStartAndHeight(2 * startSeq - endSeq, getViewportHeight()); + } + + /** + * Adjust sequence position for page down. Fires a property change event. + */ + public void pageDown() + { + setViewportStartAndHeight(endSeq, getViewportHeight()); + } } diff --git a/test/jalview/viewmodel/OverviewDimensionsTest.java b/test/jalview/viewmodel/OverviewDimensionsTest.java index 398fec3..42b9289 100644 --- a/test/jalview/viewmodel/OverviewDimensionsTest.java +++ b/test/jalview/viewmodel/OverviewDimensionsTest.java @@ -563,11 +563,11 @@ public class OverviewDimensionsTest assertEquals(od.getBoxHeight(), boxHeight); // move viewport to bottom right - moveViewport(98, 508); + moveViewport(98, 507); assertEquals(od.getBoxX(), Math.round((float) 98 * od.getWidth() / alwidth)); assertEquals(od.getBoxY(), - Math.round((float) 508 * od.getSequencesHeight() / alheight)); + Math.round((float) 507 * od.getSequencesHeight() / alheight)); assertEquals(od.getBoxWidth(), boxWidth); assertEquals(od.getBoxHeight(), boxHeight); } @@ -998,9 +998,7 @@ public class OverviewDimensionsTest // normally vpranges.setStartRes(od.getScrollCol()); - vpranges.setEndRes(od.getScrollCol() + viewWidth - 1); vpranges.setStartSeq(od.getScrollRow()); - vpranges.setEndSeq(od.getScrollRow() + viewHeight - 1); od.setBoxPosition(al.getHiddenSequences(), hiddenCols, vpranges); } diff --git a/test/jalview/viewmodel/ViewportRangesTest.java b/test/jalview/viewmodel/ViewportRangesTest.java index cfd03cd..2465c4d 100644 --- a/test/jalview/viewmodel/ViewportRangesTest.java +++ b/test/jalview/viewmodel/ViewportRangesTest.java @@ -1,10 +1,16 @@ package jalview.viewmodel; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; import jalview.analysis.AlignmentGenerator; import jalview.datamodel.AlignmentI; +import java.beans.PropertyChangeEvent; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + import org.testng.annotations.Test; public class ViewportRangesTest { @@ -13,7 +19,7 @@ public class ViewportRangesTest { AlignmentI al = gen.generate(20, 30, 1, 5, 5); - @Test + @Test(groups = { "Functional" }) public void testViewportRanges() { ViewportRanges vr = new ViewportRanges(al); @@ -24,7 +30,7 @@ public class ViewportRangesTest { assertEquals(vr.getEndSeq(), al.getHeight() - 1); } - @Test + @Test(groups = { "Functional" }) public void testGetAbsoluteAlignmentHeight() { ViewportRanges vr = new ViewportRanges(al); @@ -35,28 +41,25 @@ public class ViewportRangesTest { assertEquals(vr.getAbsoluteAlignmentHeight(), al.getHeight() + 1); } - @Test + @Test(groups = { "Functional" }) public void testGetAbsoluteAlignmentWidth() { ViewportRanges vr = new ViewportRanges(al); assertEquals(vr.getAbsoluteAlignmentWidth(), al.getWidth()); } - @Test + @Test(groups = { "Functional" }) public void testSetEndRes() { ViewportRanges vr = new ViewportRanges(al); vr.setEndRes(-1); assertEquals(vr.getEndRes(), 0); - vr.setEndRes(al.getWidth()); - assertEquals(vr.getEndRes(), al.getWidth() - 1); - vr.setEndRes(al.getWidth() - 1); assertEquals(vr.getEndRes(), al.getWidth() - 1); } - @Test + @Test(groups = { "Functional" }) public void testSetEndSeq() { ViewportRanges vr = new ViewportRanges(al); @@ -70,7 +73,7 @@ public class ViewportRangesTest { assertEquals(vr.getEndSeq(), al.getHeight() - 1); } - @Test + @Test(groups = { "Functional" }) public void testSetStartRes() { ViewportRanges vr = new ViewportRanges(al); @@ -84,7 +87,7 @@ public class ViewportRangesTest { assertEquals(vr.getStartRes(), al.getWidth() - 1); } - @Test + @Test(groups = { "Functional" }) public void testSetStartSeq() { ViewportRanges vr = new ViewportRanges(al); @@ -97,4 +100,338 @@ public class ViewportRangesTest { vr.setStartSeq(al.getHeight() - 1); assertEquals(vr.getStartSeq(), al.getHeight() - 1); } + + @Test(groups = { "Functional" }) + public void testSetStartEndRes() + { + ViewportRanges vr = new ViewportRanges(al); + vr.setStartEndRes(-1, -1); + assertEquals(vr.getStartRes(), 0); + assertEquals(vr.getEndRes(), 0); + + vr.setStartEndRes(5, 19); + assertEquals(vr.getStartRes(), 5); + assertEquals(vr.getEndRes(), 19); + + vr.setStartEndRes(al.getWidth(), al.getWidth()); + assertEquals(vr.getEndRes(), al.getWidth()); + } + + @Test(groups = { "Functional" }) + public void testSetStartEndSeq() + { + ViewportRanges vr = new ViewportRanges(al); + vr.setStartEndSeq(-1, -1); + assertEquals(vr.getStartSeq(), 0); + assertEquals(vr.getEndSeq(), 0); + + vr.setStartEndSeq(5, 19); + assertEquals(vr.getStartSeq(), 5); + assertEquals(vr.getEndSeq(), 19); + + vr.setStartEndSeq(al.getHeight(), al.getHeight()); + assertEquals(vr.getEndSeq(), al.getHeight() - 1); + } + + @Test(groups = { "Functional" }) + public void testSetViewportHeight() + { + ViewportRanges vr = new ViewportRanges(al); + vr.setViewportHeight(13); + assertEquals(vr.getViewportHeight(), 13); + } + + @Test(groups = { "Functional" }) + public void testSetViewportWidth() + { + ViewportRanges vr = new ViewportRanges(al); + vr.setViewportWidth(13); + assertEquals(vr.getViewportWidth(), 13); + } + + @Test(groups = { "Functional" }) + public void testSetViewportStartAndHeight() + { + ViewportRanges vr = new ViewportRanges(al); + vr.setViewportStartAndHeight(2, 6); + assertEquals(vr.getViewportHeight(), 6); + assertEquals(vr.getStartSeq(), 2); + + // reset -ve values of start to 0 + vr.setViewportStartAndHeight(-1, 7); + assertEquals(vr.getViewportHeight(), 7); + assertEquals(vr.getStartSeq(), 0); + + // reset out of bounds start values to within bounds + vr.setViewportStartAndHeight(35, 5); + assertEquals(vr.getViewportHeight(), 5); + assertEquals(vr.getStartSeq(), 24); + } + + @Test(groups = { "Functional" }) + public void testSetViewportStartAndWidth() + { + ViewportRanges vr = new ViewportRanges(al); + vr.setViewportStartAndWidth(2, 6); + assertEquals(vr.getViewportWidth(), 6); + assertEquals(vr.getStartRes(), 2); + + // reset -ve values of start to 0 + vr.setViewportStartAndWidth(-1, 7); + assertEquals(vr.getViewportWidth(), 7); + assertEquals(vr.getStartRes(), 0); + + // reset out of bounds start values to within bounds + vr.setViewportStartAndWidth(35, 5); + assertEquals(vr.getViewportWidth(), 5); + assertEquals(vr.getStartRes(), 20); + } + + @Test(groups = { "Functional" }) + public void testPageUpDown() + { + ViewportRanges vr = new ViewportRanges(al); + vr.setViewportStartAndHeight(8, 6); + vr.pageDown(); + assertEquals(vr.getStartSeq(), 13); + + vr.pageUp(); + assertEquals(vr.getStartSeq(), 8); + + vr.pageUp(); + assertEquals(vr.getStartSeq(), 3); + + vr.pageUp(); + // pageup does not go beyond 0, viewport height stays the same + assertEquals(vr.getStartSeq(), 0); + assertEquals(vr.getViewportHeight(), 6); + + vr.pageDown(); + vr.pageDown(); + vr.pageDown(); + vr.pageDown(); + vr.pageDown(); + + // pagedown to bottom does not go beyond end, and height stays same + assertEquals(vr.getStartSeq(), 23); + assertEquals(vr.getViewportHeight(), 6); + } + + @Test(groups = { "Functional" }) + public void testScrollUp() + { + ViewportRanges vr = new ViewportRanges(al); + vr.setViewportStartAndHeight(1, 5); + vr.scrollUp(true); + assertEquals(vr.getStartSeq(), 0); + // can't scroll above top + vr.scrollUp(true); + assertEquals(vr.getStartSeq(), 0); + + vr.setViewportStartAndHeight(23, 5); + vr.scrollUp(false); + assertEquals(vr.getStartSeq(), 24); + // can't scroll beyond bottom + vr.scrollUp(false); + assertEquals(vr.getStartSeq(), 24); + } + + @Test(groups = { "Functional" }) + public void testScrollRight() + { + ViewportRanges vr = new ViewportRanges(al); + vr.setViewportStartAndWidth(1, 5); + vr.scrollRight(false); + assertEquals(vr.getStartRes(), 0); + // can't scroll left past start + vr.scrollRight(false); + assertEquals(vr.getStartRes(), 0); + + vr.setViewportStartAndWidth(19, 5); + vr.scrollRight(true); + assertEquals(vr.getStartRes(), 20); + // can't scroll right past end + vr.scrollRight(true); + assertEquals(vr.getStartRes(), 20); + } + + @Test(groups = { "Functional" }) + public void testScrollToWrappedVisible() + { + ViewportRanges vr = new ViewportRanges(al); + vr.setViewportStartAndWidth(5, 10); + + vr.scrollToWrappedVisible(0); + assertEquals(vr.getStartRes(), 0); + + vr.scrollToWrappedVisible(10); + assertEquals(vr.getStartRes(), 10); + + vr.scrollToWrappedVisible(15); + assertEquals(vr.getStartRes(), 10); + } + + // leave until JAL-2388 is merged and we can do without viewport + /*@Test(groups = { "Functional" }) + public void testScrollToVisible() + { + ViewportRanges vr = new ViewportRanges(al); + vr.setViewportStartAndWidth(12,5); + vr.setViewportStartAndHeight(10,6); + vr.scrollToVisible(13,14) + + // no change + assertEquals(vr.getStartRes(), 12); + assertEquals(vr.getStartSeq(), 10); + + vr.scrollToVisible(5,6); + assertEquals(vr.getStartRes(), 5); + assertEquals(vr.getStartSeq(), 6); + + // test for hidden columns too + }*/ + + @Test(groups = { "Functional" }) + public void testEventFiring() + { + ViewportRanges vr = new ViewportRanges(al); + MockPropChangeListener l = new MockPropChangeListener(vr); + List emptylist = new ArrayList(); + + // one event fired when startRes is called with new value + vr.setStartRes(4); + assertTrue(l.verify(1, Arrays.asList("startres"))); + l.reset(); + + // no event fired for same value + vr.setStartRes(4); + assertTrue(l.verify(0, emptylist)); + l.reset(); + + vr.setEndRes(10); + assertTrue(l.verify(1, Arrays.asList("startres"))); + l.reset(); + + // no event fired for same value + vr.setEndRes(10); + assertTrue(l.verify(0, emptylist)); + l.reset(); + + vr.setStartSeq(4); + assertTrue(l.verify(1, Arrays.asList("startseq"))); + l.reset(); + + vr.setStartSeq(4); + assertTrue(l.verify(0, emptylist)); + l.reset(); + + vr.setEndSeq(10); + assertTrue(l.verify(1, Arrays.asList("startseq"))); + l.reset(); + + vr.setEndSeq(10); + assertTrue(l.verify(0, emptylist)); + l.reset(); + + vr.setStartEndRes(2, 15); + assertTrue(l.verify(1, Arrays.asList("startres"))); + l.reset(); + + vr.setStartEndRes(2, 15); + assertTrue(l.verify(0, emptylist)); + l.reset(); + + vr.setStartEndSeq(2, 15); + assertTrue(l.verify(1, Arrays.asList("startseq"))); + l.reset(); + + vr.setStartEndSeq(2, 15); + assertTrue(l.verify(0, emptylist)); + l.reset(); + + // test viewport height and width setting triggers event + vr.setViewportHeight(10); + assertTrue(l.verify(1, Arrays.asList("endseq"))); + l.reset(); + + vr.setViewportWidth(18); + assertTrue(l.verify(1, Arrays.asList("endres"))); + l.reset(); + + // already has seq start set to 2, so triggers endseq + vr.setViewportStartAndHeight(2, 16); + assertTrue(l.verify(1, Arrays.asList("endseq"))); + l.reset(); + + vr.setViewportStartAndWidth(1, 14); + assertTrue(l.verify(1, Arrays.asList("startres"))); + l.reset(); + + // test page up/down triggers event + vr.pageUp(); + assertTrue(l.verify(1, Arrays.asList("startseq"))); + l.reset(); + + vr.pageDown(); + assertTrue(l.verify(1, Arrays.asList("startseq"))); + l.reset(); + + // test scrolling triggers event + vr.scrollUp(true); + assertTrue(l.verify(1, Arrays.asList("startseq"))); + l.reset(); + + vr.scrollUp(false); + assertTrue(l.verify(1, Arrays.asList("startseq"))); + l.reset(); + + vr.scrollRight(true); + assertTrue(l.verify(1, Arrays.asList("startres"))); + l.reset(); + + vr.scrollRight(false); + assertTrue(l.verify(1, Arrays.asList("startres"))); + l.reset(); + + // TODO test scrollToVisibble once hidden columns JAL-2388 merged in + // to avoid somersaults with align viewport + /*vr.scrollToVisible(10, 10); + assertTrue(l.verify(1, Arrays.asList("startres"))); + l.reset();*/ + + vr.scrollToWrappedVisible(5); + assertTrue(l.verify(1, Arrays.asList("startres"))); + l.reset(); + } +} + +// mock listener for property change events +class MockPropChangeListener implements ViewportListenerI +{ + private int firecount = 0; + + private List events = new ArrayList(); + + public MockPropChangeListener(ViewportRanges vr) + { + vr.addPropertyChangeListener(this); + } + + @Override + public void propertyChange(PropertyChangeEvent evt) + { + firecount++; + events.add(evt.getPropertyName()); + } + + public boolean verify(int count, List eventslist) + { + return (count == firecount) && events.equals(eventslist); + } + + public void reset() + { + firecount = 0; + events.clear(); + } } -- 1.7.10.2