Skip to content

Commit 7c0ce63

Browse files
ponderosaTXponderosaTX
committed
This revision patches am embarrassing bug in the ReplaceAll command
that I introduced when I tried to refactor the code based on apparent bugs for some corner cases. I misunderstood some of the existing code. This revision hopefully fixes the bug I introduced as well as the old corner case bugs. The unit test suite for Find/Replace is grossly inadequate because much of the logic is embedded in the FindReplacePanel class which like most of DrJava's UI classes is poorly covered. The following classes were modified, added, or removed: modified: src/edu/rice/cs/drjava/model/FindReplaceMachine.java modified: src/edu/rice/cs/drjava/model/FindReplaceMachineTest.java modified: src/edu/rice/cs/drjava/model/GlobalModelJUnitTest.java modified: src/edu/rice/cs/drjava/model/junit/DefaultJUnitModel.java modified: src/edu/rice/cs/drjava/model/junit/JUnitError.java modified: src/edu/rice/cs/drjava/model/repl/newjvm/MainJVM.java modified: src/edu/rice/cs/drjava/ui/FindReplacePanel.java modified: src/edu/rice/cs/drjava/ui/JUnitPanel.java
1 parent 6443497 commit 7c0ce63

File tree

8 files changed

+163
-129
lines changed

8 files changed

+163
-129
lines changed

‎drjava/src/edu/rice/cs/drjava/model/FindReplaceMachine.java‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,11 @@
4444
import edu.rice.cs.util.StringOps;
4545
import edu.rice.cs.drjava.config.OptionConstants;
4646

47+
import java.awt.Component;
4748
import java.awt.EventQueue;
4849

4950
import javax.swing.text.BadLocationException;
50-
import java.awt.Component;
51+
import javax.swing.text.BadLocationException;;
5152

5253
/** Implementation of logic of the find/replace command over a given document or all open documents.
5354
* @version $Id$
@@ -230,7 +231,7 @@ public boolean onMatch() {
230231
*/
231232
public boolean replaceCurrent() {
232233

233-
assert EventQueue.isDispatchThread();
234+
assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
234235

235236
if (! onMatch()) return false;
236237
try {
@@ -275,7 +276,7 @@ public boolean replaceCurrent() {
275276
// */
276277
// private int replaceAll(boolean searchAll, boolean searchSelectionOnly) {
277278
//
278-
// assert EventQueue.isDispatchThread();
279+
// assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
279280
//
280281
// if (searchAll) {
281282
// int count = 0; // the number of replacements done so far
@@ -316,7 +317,7 @@ public boolean replaceCurrent() {
316317
// */
317318
// private int _replaceAllInCurrentDoc(boolean searchSelectionOnly) {
318319
//
319-
// assert EventQueue.isDispatchThread();
320+
// assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
320321
//
321322
// if (! searchSelectionOnly) {
322323
// _selectionRegion = new MovingDocumentRegion(_doc, 0, _doc.getLength(),
@@ -344,8 +345,8 @@ public boolean replaceCurrent() {
344345

345346
/** Replaces all occurrences of the find word with the replace word in the specified region while performing the
346347
* which occurs within the current document. On each match, the findAction command is executed, assuming that
347-
* indAction does not modify the document it processes. Saves value of _searchAllDocuments and _searchSelectionOnly
348-
* and restores trem, an ugly hack dictated by embedding this information in the FindReplaceMachine. During this
348+
* findAction does not modify the document it processes. Saves value of _searchAllDocuments and _searchSelectionOnly
349+
* and restores them, an ugly hack dictated by embedding this information in the FindReplaceMachine. During this
349350
* particular search action, _searchAllDocuments is false since it is confined to a region within the current document.
350351
* Only executes in event thread.
351352
* @param findAction action to perform on the occurrences; input is the FindResult, output is ignored
@@ -354,7 +355,7 @@ public boolean replaceCurrent() {
354355
*/
355356
public int processAll(Runnable1<FindResult> findAction, MovingDocumentRegion region) {
356357

357-
assert EventQueue.isDispatchThread();
358+
assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
358359

359360
_selectionRegion = region;
360361

@@ -373,7 +374,7 @@ public int processAll(Runnable1<FindResult> findAction, MovingDocumentRegion reg
373374
*/
374375
public int processAll(Runnable1<FindResult> findAction) {
375376

376-
assert EventQueue.isDispatchThread();
377+
assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
377378

378379
if (_searchAllDocuments) {
379380
int count = 0; // the number of replacements done so far
@@ -416,7 +417,7 @@ else if (_searchSelectionOnly) {
416417
*/
417418
private int _processAllInCurrentDoc(Runnable1<FindResult> findAction) {
418419

419-
assert EventQueue.isDispatchThread();
420+
assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
420421

421422
if (! _searchSelectionOnly) {
422423
_selectionRegion = new MovingDocumentRegion(_doc, 0, _doc.getLength(), _doc._getLineStartPos(0),
@@ -449,7 +450,7 @@ private int _processAllInCurrentDoc(Runnable1<FindResult> findAction) {
449450
*/
450451
private FindResult findNext(boolean searchAll) {
451452

452-
assert EventQueue.isDispatchThread();
453+
assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
453454

454455
// Find next match, if any, in _doc.
455456
FindResult fr;
@@ -500,7 +501,7 @@ private FindResult findNext(boolean searchAll) {
500501
*/
501502
private FindResult _findNextInDoc(OpenDefinitionsDocument doc, int start, int len, boolean searchAll) {
502503

503-
assert EventQueue.isDispatchThread();
504+
assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
504505

505506
// search from current position to "end" of document ("end" is start if searching backward)
506507
// Utilities.show("_findNextInDoc([" + doc.getText() + "], " + start + ", " + len + ", " + searchAll + ")");
@@ -522,7 +523,7 @@ private FindResult _findNextInDoc(OpenDefinitionsDocument doc, int start, int le
522523
*/
523524
private FindResult _findWrapped(OpenDefinitionsDocument doc, int start, int len, boolean allWrapped) {
524525

525-
assert EventQueue.isDispatchThread();
526+
assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
526527

527528
final int docLen = doc.getLength();
528529
if (docLen == 0) return new FindResult(doc, -1, true, allWrapped); // failure result
@@ -582,7 +583,7 @@ private FindResult _findNextInDocSegment(final OpenDefinitionsDocument doc, fina
582583
// Utilities.show("called _findNextInDocSegment(" + doc.getText() + ",\n" + start + ", " + len + ", " + wrapped +
583584
// ", " = allWrapped + ")");
584585

585-
assert EventQueue.isDispatchThread();
586+
assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
586587

587588
boolean inTestCase = false;
588589
for(String ext: OptionConstants.LANGUAGE_LEVEL_EXTENSIONS) {
@@ -678,7 +679,7 @@ private FindResult _findNextInDocSegment(final OpenDefinitionsDocument doc, fina
678679
*/
679680
private FindResult _findNextInOtherDocs(final OpenDefinitionsDocument startDoc, int start, int len) {
680681

681-
assert EventQueue.isDispatchThread();
682+
assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
682683

683684
// System.err.println("_findNextInOtherDocs(" + startDoc.getText() + ", " + start + ", " + len + ")");
684685

@@ -723,7 +724,7 @@ private FindResult _findNextInOtherDocs(final OpenDefinitionsDocument startDoc,
723724
*/
724725
private boolean wholeWordFoundAtCurrent(OpenDefinitionsDocument doc, int foundOffset) {
725726

726-
assert EventQueue.isDispatchThread();
727+
assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
727728

728729
char leftOfMatch = 0; // forced initialization
729730
char rightOfMatch = 0; // forced initialization
@@ -761,7 +762,7 @@ private boolean wholeWordFoundAtCurrent(OpenDefinitionsDocument doc, int foundOf
761762
*/
762763
private boolean _shouldIgnore(int foundOffset, OpenDefinitionsDocument odd) {
763764

764-
assert EventQueue.isDispatchThread();
765+
assert EventQueue.isDispatchThread() || Utilities.TEST_MODE;
765766

766767
return (_matchWholeWord && ! wholeWordFoundAtCurrent(odd, foundOffset)) ||
767768
(_ignoreCommentsAndStrings && odd.isShadowed(foundOffset));

‎drjava/src/edu/rice/cs/drjava/model/FindReplaceMachineTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,10 @@ public void testSimpleReplace() throws BadLocationException {
229229
_initFrm(0);
230230
_assertOffsets(_frm, 0, 0);
231231
_frm.setFindWord("evil");
232-
_frm.setReplaceWord("monkey");
232+
_frm.setReplaceWord("monkey evil");
233233
_testFindNextSucceeds(_frm, 0, 12);
234234
Utilities.invokeAndWait(new Runnable() { public void run() { _frm.replaceCurrent(); } });
235-
assertEquals("new replaced text", "Hear no monkey, see no evil, speak no evil.", _doc.getText());
235+
assertEquals("new replaced text", "Hear no monkey evil, see no evil, speak no evil.", _doc.getText());
236236
// System.err.println("testSimpleReplace completed");
237237
}
238238

@@ -242,9 +242,9 @@ public void testReplaceAllContinue() throws BadLocationException {
242242
_initFrm(15);
243243
_assertOffsets(_frm, 15, 15);
244244
_frm.setFindWord("evil");
245-
_frm.setReplaceWord("monkey");
245+
_frm.setReplaceWord("monkey evil");
246246
replaceAll();
247-
assertEquals("revised text", "Hear no monkey, see no monkey, speak no monkey.", _doc.getText());
247+
assertEquals("revised text", "Hear no monkey evil, see no monkey evil, speak no monkey evil.", _doc.getText());
248248
// System.err.println("testReplaceAllContinue completed");
249249
}
250250

‎drjava/src/edu/rice/cs/drjava/model/GlobalModelJUnitTest.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -254,9 +254,9 @@ public void testElspethOneJUnitError_NOJOIN() throws Exception {
254254

255255
listener.runJUnit(doc);
256256

257-
JUnitErrorModel jem = _model.getJUnitModel().getJUnitErrorModel();
258-
assertEquals("test case has one error reported", 1, jem.getNumErrors());
259-
assertTrue("first error should be an error not a warning", !jem.getError(0).isWarning());
257+
JUnitErrorModel junitErrorModel = _model.getJUnitModel().getJUnitErrorModel();
258+
assertEquals("test case has one error reported", 1, junitErrorModel.getNumErrors());
259+
assertTrue("first error should be an error not a warning", !junitErrorModel.getError(0).isWarning());
260260
_model.removeListener(listener);
261261

262262
_log.log("testElspethOneJUnitError completed");
@@ -285,14 +285,14 @@ public void testRealError_NOJOIN() throws Exception {
285285
listener.assertJUnitEndCount(1);
286286
_model.removeListener(listener);
287287

288-
_log.log("testRealError completed");
288+
_log.log("+++Completing testRealError completed");
289289
}
290290

291291
/** Tests that the ui is notified to put up an error dialog if JUnit is run on a non-TestCase.
292292
* @throws Exception if something goes wrong
293293
*/
294294
public void testNonTestCaseError_NOJOIN() throws Exception {
295-
_log.log("----testNonTestCaseError-----");
295+
_log.log("+++Starting testNonTestCaseError");
296296
// Utilities.show("Running testNonTestCaseError");
297297

298298
final OpenDefinitionsDocument doc = setupDocument(NON_TESTCASE_TEXT);
@@ -674,11 +674,11 @@ public void safeJUnitAllWithErrors() throws Exception {
674674
listener.assertJUnitTestEndedCount(2);
675675
_model.removeListener(listener);
676676

677-
JUnitErrorModel jem = _model.getJUnitModel().getJUnitErrorModel();
678-
assertEquals("test case has one error reported", 2, jem.getNumErrors());
677+
JUnitErrorModel junitErrorModel = _model.getJUnitModel().getJUnitErrorModel();
678+
assertEquals("test case has one error reported", 2, junitErrorModel.getNumErrors());
679679

680-
assertTrue("first error should be an error", jem.getError(0).isWarning());
681-
assertFalse("second error should be a failure", jem.getError(1).isWarning());
680+
assertTrue("first error should be an error", junitErrorModel.getError(0).isWarning());
681+
assertFalse("second error should be a failure", junitErrorModel.getError(1).isWarning());
682682

683683
_log.log("testJUnitAllWithErrors completed");
684684
}

‎drjava/src/edu/rice/cs/drjava/model/junit/DefaultJUnitModel.java‎

Lines changed: 41 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,12 @@
4848
import java.util.HashSet;
4949
import java.util.Set;
5050
import edu.rice.cs.drjava.config.BooleanOption;
51-
import edu.rice.cs.drjava.model.GlobalModel;
51+
import edu.rice.cs.drjava.model.DrJavaFileUtils;
5252
import edu.rice.cs.drjava.model.FileMovedException;
53+
import edu.rice.cs.drjava.model.FindReplaceMachine;
54+
import edu.rice.cs.drjava.model.FindResult;
5355
import edu.rice.cs.drjava.model.OpenDefinitionsDocument;
54-
import edu.rice.cs.drjava.model.DrJavaFileUtils;
56+
import edu.rice.cs.drjava.model.SingleDisplayModel;
5557
import edu.rice.cs.drjava.model.repl.newjvm.MainJVM;
5658
import edu.rice.cs.drjava.model.compiler.CompilerModel;
5759
import edu.rice.cs.drjava.model.compiler.CompilerListener;
@@ -85,7 +87,7 @@ public class DefaultJUnitModel implements JUnitModel, JUnitModelCallback {
8587
private CoverageMetadata coverageMetadata = new CoverageMetadata(false, "");
8688

8789
/** log for use in debugging */
88-
private static Log _log = new Log("DefaultJUnitModel.txt", false);
90+
private static Log _log = new Log("GlobalModel.txt", false);
8991

9092
/** Manages listeners to this model. */
9193
private final JUnitEventNotifier _notifier = new JUnitEventNotifier();
@@ -101,7 +103,7 @@ public class DefaultJUnitModel implements JUnitModel, JUnitModelCallback {
101103
private final CompilerModel _compilerModel;
102104

103105
/** The global model to which the JUnitModel belongs */
104-
private final GlobalModel _model;
106+
private final SingleDisplayModel _model;
105107

106108
/** The error model containing all current JUnit errors. */
107109
private volatile JUnitErrorModel _junitErrorModel;
@@ -122,7 +124,7 @@ public class DefaultJUnitModel implements JUnitModel, JUnitModelCallback {
122124
* @param compilerModel the CompilerModel, used only as a lock to prevent simultaneous test and compile
123125
* @param model used only for getSourceFile
124126
*/
125-
public DefaultJUnitModel(MainJVM jvm, CompilerModel compilerModel, GlobalModel model) {
127+
public DefaultJUnitModel(MainJVM jvm, CompilerModel compilerModel, SingleDisplayModel model) {
126128
_jvm = jvm;
127129
_compilerModel = compilerModel;
128130
_model = model;
@@ -191,39 +193,20 @@ public void junitProject() {
191193
junitOpenDefDocs(lod, true);
192194
}
193195

194-
// /** Forwards the classnames and files to the test manager to test all of them; does not notify
195-
// * since we don't have ODD's to send out with the notification of junit start.
196-
// * @param qualifiedClassnames a list of all the qualified class names to test.
197-
// * @param files a list of their source files in the same order as qualified class names.
198-
// */
199-
// public void junitClasses(List<String> qualifiedClassnames, List<File> files) {
200-
// Utilities.showDebug("junitClasses(" + qualifiedClassnames + ", " + files);
201-
// synchronized(_compilerModel.getCompilerLock()) {
202-
//
203-
// // Check _testInProgress
204-
// if (_testInProgress) return;
205-
//
206-
// List<String> testClasses;
207-
// try { testClasses = _jvm.findTestClasses(qualifiedClassnames, files); }
208-
// catch(IOException e) { throw new UnexpectedException(e); }
209-
//
210-
//// System.err.println("Found test classes: " + testClasses);
211-
//
212-
// if (testClasses.isEmpty()) {
213-
// nonTestCase(true);
214-
// return;
215-
// }
216-
// _notifier.junitClassesStarted();
217-
// _testInProgress = true;
218-
// try { _jvm.runTestSuite(); }
219-
// catch(Exception e) {
220-
//// System.err.println("Threw exception " + e);
221-
// _notifier.junitEnded();
222-
// _testInProgress = false;
223-
// throw new UnexpectedException(e);
224-
// }
225-
// }
226-
// }
196+
/** Determines if className appears as an identifier in the open documents. */
197+
private boolean appearsInSourceText(String className, FindReplaceMachine frm) {
198+
_log.log("***appearsInSourceText(" + className + ", " + frm + ")");
199+
OpenDefinitionsDocument doc = _model.getActiveDocument();
200+
frm.setDocument(doc);
201+
frm.setFirstDoc(doc);
202+
frm.setPosition(doc.getCurrentLocation());
203+
frm.setFindWord(className);
204+
frm.setIgnoreCommentsAndStrings(true);
205+
frm.setSearchAllDocuments(true);
206+
FindResult match = frm.findNext();
207+
_log.log("Matching result = " + match);
208+
return (match.getFoundOffset() != -1);
209+
}
227210

228211
public void junitDocs(List<OpenDefinitionsDocument> lod) { junitOpenDefDocs(lod, true); }
229212

@@ -384,6 +367,12 @@ private void _rawJUnitOpenDefDocs(List<OpenDefinitionsDocument> lod, final boole
384367
/* Source files corresonding to potential test class files */
385368
final ArrayList<File> files = new ArrayList<File>();
386369

370+
/* Flag indicating if project is open */
371+
final boolean isProject = _model.isProjectActive();
372+
373+
/* Set up FindReplaceMachine to search open definitions documents. */
374+
final FindReplaceMachine frm = new FindReplaceMachine(_model, _model.getDocumentIterator(), null);
375+
387376
try {
388377
for (File dir: classDirs) { // foreach class file directory
389378
// System.err.println("Examining directory " + dir);
@@ -395,25 +384,27 @@ private void _rawJUnitOpenDefDocs(List<OpenDefinitionsDocument> lod, final boole
395384
if (listing != null) { // listFiles may return null if there's an IO error
396385
for (File entry : listing) { /* for each class file in the build directory */
397386

398-
//System.err.println("Examining file " + entry);
387+
_log.log("Examining file " + entry);
399388

400389
/* ignore non-class files */
401-
String name = entry.getName();
390+
final String name = entry.getName();
402391
if (! name.endsWith(".class")) continue;
403392

393+
final String noExtName = name.substring(0, name.length() - 6); // remove ".class" from name
394+
final int indexOfLastDot = noExtName.lastIndexOf('.');
395+
final String simpleClassName = noExtName.substring(indexOfLastDot + 1);
396+
_log.log("Simple class name is " + simpleClassName);
397+
404398
/* Ignore class names that do not end in "Test" if FORCE_TEST_SUFFIX option is set */
405-
String noExtName = "";
406-
if (_forceTestSuffix) {
407-
noExtName = name.substring(0, name.length() - 6); // remove ".class" from name
408-
int indexOfLastDot = noExtName.lastIndexOf('.');
409-
String simpleClassName = noExtName.substring(indexOfLastDot + 1);
410-
// System.err.println("Simple class name is " + simpleClassName);
411-
if (/*isProject &&*/ ! simpleClassName.endsWith("Test")) continue;
412-
}
399+
if (_forceTestSuffix && ! simpleClassName.endsWith("Test")) continue;
413400

414401
/* ignore entries that do not correspond to files? Can this happen? */
415402
if (! entry.isFile()) continue;
416403

404+
/* In flat file mode, ignore files that are not named in a source document. */
405+
if (! isProject && ! appearsInSourceText(simpleClassName, frm)) continue;
406+
_log.log("isProject = " + isProject + "; name = " + name);
407+
417408
// Add this class and the corrresponding source file to classNames and files, respectively.
418409
// Finding the source file is non-trivial because it may be a language-levels file
419410

@@ -424,9 +415,7 @@ private void _rawJUnitOpenDefDocs(List<OpenDefinitionsDocument> lod, final boole
424415
public void visit(int version, int access, String name, String sig, String sup, String[] inters) {
425416
className.set(name.replace('/', '.'));
426417
}
427-
public void visitSource(String source, String debug) {
428-
sourceName.set(source);
429-
}
418+
public void visitSource(String source, String debug) { sourceName.set(source); }
430419
public void visitOuterClass(String owner, String name, String desc) { }
431420
public AnnotationVisitor visitAnnotation(String desc, boolean visible) { return null; }
432421
public void visitAttribute(Attribute attr) { }
@@ -660,7 +649,7 @@ public File getFileForClassName(String className) {
660649
/** Returns the current classpath in use by the JUnit JVM. */
661650
public Iterable<File> getClassPath() { return _jvm.getClassPath().unwrap(IterUtil.<File>empty()); }
662651

663-
/** Called when the JVM used for unit tests has registered. Does not necessarily run in even thread. */
652+
/** Called when the JVM used for unit tests has registered. Does not necessarily run in event thread. */
664653
public void junitJVMReady() {
665654
Utilities.invokeLater(new Runnable() { public void run() {
666655
if (! _testInProgress) return;

‎drjava/src/edu/rice/cs/drjava/model/junit/JUnitError.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public JUnitError(File file, int lineNumber, int startColumn, String message, bo
7777
* @param test the name of the test that failed
7878
*/
7979
public JUnitError(String message, boolean isWarning, String test) {
80-
this(null, -1, -1, message, isWarning, test, "", "No associated stack trace", new StackTraceElement[0]);
80+
this(null, -1, -1, message, isWarning, test, "", "Internal DrJava JUnitError", Thread.currentThread().getStackTrace());
8181
}
8282

8383
/** Gets the test name

0 commit comments

Comments
 (0)