From 574211a28b085a4bd67fe7a161cbd9773ecb75cf Mon Sep 17 00:00:00 2001 From: gmungoc Date: Thu, 14 Sep 2017 14:44:42 +0100 Subject: [PATCH] JAL-2541 it works!! it works!!! --- src/jalview/commands/EditCommand.java | 10 ++- test/jalview/commands/EditCommandTest.java | 128 ++++++++++++++++++++-------- 2 files changed, 98 insertions(+), 40 deletions(-) diff --git a/src/jalview/commands/EditCommand.java b/src/jalview/commands/EditCommand.java index 6484224..9be8d3f 100644 --- a/src/jalview/commands/EditCommand.java +++ b/src/jalview/commands/EditCommand.java @@ -753,8 +753,7 @@ public class EditCommand implements CommandI tmp.append(oldstring.substring(end)); command.seqs[i].setSequence(tmp.toString()); command.string[i] = oldstring.substring(start, end).toCharArray(); - String nogapold = jalview.analysis.AlignSeq.extractGaps( - jalview.util.Comparison.GapChars, + String nogapold = AlignSeq.extractGaps(Comparison.GapChars, new String(command.string[i])); if (!nogaprep.toLowerCase().equals(nogapold.toLowerCase())) { @@ -903,8 +902,7 @@ public class EditCommand implements CommandI } // and then duplicate added annotation on every other alignment // view - for (int vnum = 0; views != null - && vnum < views.length; vnum++) + for (int vnum = 0; views != null && vnum < views.length; vnum++) { if (views[vnum] != command.al) { @@ -1461,6 +1459,10 @@ public class EditCommand implements CommandI protected static void cutFeatures(Edit command, SequenceI seq, int fromPosition, int toPosition, boolean cutIsInternal) { + if (!cutIsInternal) + { + return; + } List added = new ArrayList<>(); List removed = new ArrayList<>(); diff --git a/test/jalview/commands/EditCommandTest.java b/test/jalview/commands/EditCommandTest.java index a415338..111adba 100644 --- a/test/jalview/commands/EditCommandTest.java +++ b/test/jalview/commands/EditCommandTest.java @@ -34,6 +34,8 @@ import jalview.datamodel.SequenceI; import jalview.datamodel.features.SequenceFeatures; import jalview.gui.JvOptionPane; +import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.Map; @@ -50,6 +52,22 @@ import org.testng.annotations.Test; */ public class EditCommandTest { + private static Comparator BY_DESCRIPTION = new Comparator() + { + + @Override + public int compare(SequenceFeature o1, SequenceFeature o2) + { + return o1.getDescription().compareTo(o2.getDescription()); + } + }; + + private EditCommand testee; + + private SequenceI[] seqs; + + private Alignment al; + /* * compute n(n+1)/2 e.g. * func(5) = 5 + 4 + 3 + 2 + 1 = 15 @@ -66,12 +84,6 @@ public class EditCommandTest JvOptionPane.setMockResponse(JvOptionPane.CANCEL_OPTION); } - private EditCommand testee; - - private SequenceI[] seqs; - - private Alignment al; - @BeforeMethod(alwaysRun = true) public void setUp() { @@ -762,48 +774,88 @@ public class EditCommandTest { for (int to = from; to < seq0.getLength(); to++) { - testee.appendEdit(Action.CUT, sqs, from, (to - from + 1), - alignment, true); + EditCommand ec = new EditCommand("Cut", Action.CUT, sqs, from, (to + - from + 1), alignment); + final String msg = String.format("Cut %d-%d ", from + 1, to + 1); - sfs = seq0.getSequenceFeatures(); + verifyCut(seq0, from, to, msg); /* - * confirm the number of features has reduced by the - * number of features within the cut region i.e. by - * func(length of cut) + * undo and verify */ - String msg = String.format("Cut %d-%d ", from + 1, to + 1); - if (to - from == 4) - { - // all columns were cut - assertTrue(sfs.isEmpty()); - } - else - { - assertEquals(msg + "wrong number of features left", func(5) - - func(to - from + 1), sfs.size()); - } + AlignmentI[] views = new AlignmentI[] { alignment }; + ec.undoCommand(views); + sfs = seq0.getSequenceFeatures(); + assertEquals("After undo of " + msg, func(5), sfs.size()); + verifyUndo(from, to, sfs); /* - * inspect individual features + * redo and verify */ - for (SequenceFeature sf : sfs) - { - checkFeatureRelocation(sf, from + 1, to + 1, from > 0); - } + ec.doCommand(views); + verifyCut(seq0, from, to, msg); /* * undo ready for next cut */ - testee.undoCommand(new AlignmentI[] { alignment }); - sfs = seq0.getSequenceFeatures(); - assertEquals("After undo of " + msg, func(5), sfs.size()); - verifyUndo(from, to, sfs); + ec.undoCommand(views); } } } /** + * Verify by inspection that the sequence features left on the sequence after + * a cut match the expected results. The trick to this is that we can parse + * each feature's original start-end positions from its description. + * + * @param seq0 + * @param from + * @param to + * @param msg + */ + protected void verifyCut(SequenceI seq0, int from, int to, + final String msg) + { + List sfs; + sfs = seq0.getSequenceFeatures(); + + Collections.sort(sfs, BY_DESCRIPTION); + + /* + * confirm the number of features has reduced by the + * number of features within the cut region i.e. by + * func(length of cut); exception is a cut at start or end of sequence, + * which retains the original coordinates, dataset sequence + * and all its features + */ + boolean datasetRetained = from == 0 || to == 4; + if (datasetRetained) + { + // dataset and all features retained + assertEquals(msg, func(5), sfs.size()); + } + else if (to - from == 4) + { + // all columns were cut + assertTrue(sfs.isEmpty()); + } + else + { + // failure in checkFeatureRelocation is more informative! + assertEquals(msg + "wrong number of features left", func(5) + - func(to - from + 1), sfs.size()); + } + + /* + * inspect individual features + */ + for (SequenceFeature sf : sfs) + { + verifyFeatureRelocation(sf, from + 1, to + 1, !datasetRetained); + } + } + + /** * Check that after Undo, every feature has start/end that match its original * "start" and "end" properties * @@ -835,7 +887,7 @@ public class EditCommandTest * end of cut (last residue cut) * @param newDataset */ - private void checkFeatureRelocation(SequenceFeature sf, int from, int to, + private void verifyFeatureRelocation(SequenceFeature sf, int from, int to, boolean newDataset) { // TODO handle the gapped sequence case as well @@ -846,7 +898,13 @@ public class EditCommandTest String msg = String.format( "Feature %s relocated to %d-%d after cut of %d-%d", sf.getDescription(), sf.getBegin(), sf.getEnd(), from, to); - if (oldTo < from) + if (!newDataset) + { + // dataset retained with all features unchanged + assertEquals("0: " + msg, oldFrom, sf.getBegin()); + assertEquals("0: " + msg, oldTo, sf.getEnd()); + } + else if (oldTo < from) { // before cut region so unchanged assertEquals("1: " + msg, oldFrom, sf.getBegin()); @@ -916,7 +974,5 @@ public class EditCommandTest SequenceFeature sf = sfs.get(0); assertEquals(10, sf.getBegin()); assertEquals(11, sf.getEnd()); - - // TODO add further cases including Undo - see JAL-2541 } } -- 1.7.10.2