Skip to content

Commit 7d4afa7

Browse files
ponderosaTXponderosaTX
committed
This revision fixes a subtle bug in JUnit testing programs in flat
file mode. In this mode, the search for compiled test files inadvertently includes all class files located in thes same directories as class files corresponding to the open progam. This revision only tests the class files whose names appear as identifiers in open documents. The following files were modified, added or deleted modified: src/edu/rice/cs/drjava/model/AbstractGlobalModel.java modified: src/edu/rice/cs/drjava/model/FindReplaceMachine.java modified: src/edu/rice/cs/drjava/model/FindResult.java modified: src/edu/rice/cs/drjava/model/GlobalModel.java modified: src/edu/rice/cs/drjava/model/GlobalModelCompileErrorsTest.java modified: src/edu/rice/cs/drjava/model/GlobalModelCompileIOTest.java modified: src/edu/rice/cs/drjava/model/GlobalModelCompileSuccessOptionsTest.java modified: src/edu/rice/cs/drjava/model/GlobalModelIOTest.java modified: src/edu/rice/cs/drjava/model/GlobalModelJUnitTest.java modified: src/edu/rice/cs/drjava/model/GlobalModelOtherTest.java modified: src/edu/rice/cs/drjava/model/GlobalModelTestCase.java modified: src/edu/rice/cs/drjava/model/SingleDisplayModelTest.java modified: src/edu/rice/cs/drjava/model/compiler/DefaultCompilerModel.java modified: src/edu/rice/cs/drjava/model/junit/DefaultJUnitModel.java modified: src/edu/rice/cs/drjava/model/junit/JUnitTestManager.java modified: src/edu/rice/cs/drjava/model/repl/InteractionsModel.java modified: src/edu/rice/cs/drjava/ui/DefinitionsPane.java modified: src/edu/rice/cs/drjava/ui/MainFrameStatics.java
1 parent dcb0647 commit 7d4afa7

18 files changed

+209
-284
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2232,8 +2232,9 @@ public OpenDefinitionsDocument getNextDocument(OpenDefinitionsDocument d, Compon
22322232
private OpenDefinitionsDocument getNextDocHelper(OpenDefinitionsDocument nextdoc, Component frame) {
22332233
if ( nextdoc.isUntitled() || nextdoc.verifyExists()) return nextdoc;
22342234
// Note: verifyExists prompts user for location of the file if it is not found
2235-
int rc = JOptionPane.showConfirmDialog(frame, "Files not found, continue to next document?", "Continue?",
2236-
JOptionPane.YES_NO_OPTION);
2235+
int rc =
2236+
JOptionPane.showConfirmDialog(frame, "No valid document found (supported by a file), continue to next document?",
2237+
"Continue?", JOptionPane.YES_NO_OPTION);
22372238
if (rc == JOptionPane.NO_OPTION)
22382239
return null;
22392240
// cannot find nextdoc; move on to next document

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

Lines changed: 19 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ public class FindReplaceMachine {
7878

7979
/** Standard Constructor. Creates new machine to perform find/replace operations on documents. Only called ONCE in
8080
* initializing FindReplacePanel. Search/replace commands reuse this machine for each subsequent search/replace
81+
* NOTE: the constructor does not fully initialize the machine! You must also execute setDocument, setFirstDoc,
82+
* setPosition, and setFindWord
8183
* @param model the model
8284
* @param docIterator an object that allows navigation through open Swing documents (it is DefaultGlobalModel)
8385
* @param frame the frame
@@ -95,7 +97,7 @@ public FindReplaceMachine(SingleDisplayModel model, DocumentIterator docIterator
9597
setFindAnyOccurrence();
9698
setFindWord("");
9799
setReplaceWord("");
98-
setSearchBackwards(false);
100+
setSearchBackwards(false); // What a ridiculous way to initialize _isForward !!!
99101
setMatchCase(true);
100102
setSearchAllDocuments(false);
101103
setSearchSelectionOnly(false);
@@ -260,87 +262,6 @@ public boolean replaceCurrent() {
260262
* @param s selected region
261263
*/
262264
public void setSelection(MovingDocumentRegion s) { _selectionRegion = s; }
263-
264-
// /** Alternative interface for the private method replaceAll(...)
265-
// * @return the number of replacements */
266-
// public int replaceAll() { return replaceAll(_searchAllDocuments, _searchSelectionOnly); }
267-
//
268-
// /** Replaces all occurences of the find word with the replace word
269-
// * (i) in the current document or
270-
// * (ii) in all documents or
271-
// * (iii) in the current selection of the current document
272-
// * (depending the value of the flags searchAll and searchSelectionOnly)
273-
// * @return the number of replacements
274-
// * private method that is ONLY called in public method replaceAll()
275-
// */
276-
// private int replaceAll(boolean searchAll, boolean searchSelectionOnly) {
277-
//
278-
// assert EventQueue.isDispatchThread();
279-
//
280-
// if (searchAll) {
281-
// int count = 0; // the number of replacements done so far
282-
// int n = _docIterator.getDocumentCount();
283-
// for (int i = 0; i < n; i++) {
284-
// // replace all in the rest of the documents
285-
// count += _replaceAllInCurrentDoc(false);
286-
// _doc = _docIterator.getNextDocument(_doc, _frame);
287-
//
288-
// if (_doc == null) break;
289-
// }
290-
//
291-
// // update display (adding "*") in navigatgorPane
292-
// _model.getDocumentNavigator().repaint();
293-
//
294-
// return count;
295-
// }
296-
// else if (searchSelectionOnly) {
297-
// int count = 0;
298-
// count += _replaceAllInCurrentDoc(searchSelectionOnly);
299-
// return count;
300-
// }
301-
// else
302-
// return _replaceAllInCurrentDoc(false);
303-
// }
304-
//
305-
// /** Replaces all occurences of _findWord with _replaceWord in _doc. Never searches in other documents. Starts at
306-
// * the beginning or the end of the document (depending on find direction). This convention ensures that matches
307-
// * created by string replacement will not be replaced as in the following example:<p>
308-
// * findString: "hello"<br>
309-
// * replaceString: "e"<br>
310-
// * document text: "hhellollo"<p>
311-
// * Depending on the cursor position, clicking replace all could either make the document text read "hello"
312-
// * (which is correct) or "e". This is because of the behavior of findNext(), and it would be incorrect
313-
// * to change that behavior. Only executes in event thread.
314-
// * @param searchSelectionOnly true if we should only search in the current selection of documents
315-
// * @return the number of replacements
316-
// */
317-
// private int _replaceAllInCurrentDoc(boolean searchSelectionOnly) {
318-
//
319-
// assert EventQueue.isDispatchThread();
320-
//
321-
// if (! searchSelectionOnly) {
322-
// _selectionRegion = new MovingDocumentRegion(_doc, 0, _doc.getLength(),
323-
// _doc._getLineStartPos(0),
324-
// _doc._getLineEndPos(_doc.getLength()));
325-
// }
326-
// /* _selectionRegion is not degenerate unless document is degenerate; may be entire document. */
327-
// if (_isForward) setPosition(_selectionRegion.getStartOffset());
328-
// else setPosition(_selectionRegion.getEndOffset());
329-
//
330-
// int count = 0;
331-
// FindResult fr = findNext(false); // find next match in current doc
332-
// // Utilities.show(fr + " returned by call on findNext()");
333-
//
334-
// while (!fr.isWrapped() && fr.getFoundOffset() <= _selectionRegion.getEndOffset()) {
335-
// replaceCurrent();
336-
// count++;
337-
// // Utilities.show("Found " + count + " occurrences. Calling findNext() inside loop");
338-
// fr = findNext(false); // find next match in current doc
339-
// // Utilities.show("Call on findNext() returned " + fr.toString() + "in doc '" +
340-
// // _doc.getText().substring(0,fr.getFoundOffset()) + "[|]" + _doc.getText().substring(fr.getFoundOffset()) + "'");
341-
// }
342-
// return count;
343-
// }
344265

345266
/** Replaces all occurrences of the find word with the replace word in the specified region while performing the
346267
* which occurs within the current document. On each match, the findAction command is executed, assuming that
@@ -487,17 +408,15 @@ private FindResult findNext(boolean searchAll) {
487408
}
488409

489410

490-
/** Finds next match in specified doc only. If searching forward, len must
491-
* be doc.getLength(). If searching backward, start must be 0. If
492-
* searchAll, suppress executing in-document wrapped search, because it
493-
* must be deferred. Only runs in the event thread. Note than this method
494-
* does a wrapped search if specified search fails.
495-
* @param doc the document to search in
496-
* @param start the start offset for searching
497-
* @param len the length to search
498-
* @param searchAll flao
499-
* @return the next match in the specified document
500-
*/
411+
/** Finds next match in specified doc only. If searching forward, len must be doc.getLength(). If searching
412+
* backward, start must be 0. If searchAll, suppress executing in-document wrapped search, because it must be
413+
* deferred. Only runs in the event thread. Note than this method does a wrapped search if specified search fails.
414+
* @param doc the document to search in
415+
* @param start the start offset for searching
416+
* @param len the length to search
417+
* @param searchAll flao
418+
* @return the next match in the specified document
419+
*/
501420
private FindResult _findNextInDoc(OpenDefinitionsDocument doc, int start, int len, boolean searchAll) {
502421

503422
assert EventQueue.isDispatchThread();
@@ -615,16 +534,15 @@ private FindResult _findNextInDocSegment(final OpenDefinitionsDocument doc, fina
615534
// if (wrapped && allWrapped) Utilities.show("Executing loop with findWord = " + findWord + "; text = " + text +
616535
// "; len = " + len);
617536

618-
// loop to find first valid (not ignored) occurrence of findWord
619-
// loop carried variables are rem, foundOffset;
620-
// loop invariant variables are _doc, docLen, _isForward, findWord, wordLen, start, len.
621-
// Invariant: on forwardsearch, foundOffset + rem == len; on backward search foundOffset == rem.
622-
// loop exits by returning match (as FindResult) or by falling through with no match.
623-
// if match is returned, _current has been updated to match location
537+
/* Loop to find first valid (not ignored) occurrence of findWord. The loop-carried variables are rem,
538+
* foundOffset; the loop invariant variables are _doc, docLen, _isForward, findWord, wordLen, start, len.
539+
* Invariant: on forwardsearch, foundOffset + rem == len; on backward search foundOffset == rem.
540+
* The loop exits by returning match (as FindResult) or by falling through with no match. If match is returned,
541+
* _current has been updated to match location */
624542
int foundOffset = _isForward? 0 : len;
625543
int rem = len;
626-
// _log.log("Starting search loop; text = '" + text + "' findWord = '" + findWord + "' forward? = " + _isForward +
627-
// " rem = " + rem + " foundOffset = " + foundOffset);
544+
_log.log("Starting search loop; text = '" + text + "' findWord = '" + findWord + "' forward? = " + _isForward +
545+
" rem = " + rem + " foundOffset = " + foundOffset);
628546
while (rem >= wordLen) {
629547

630548
// Find next match in text

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@
3636

3737
package edu.rice.cs.drjava.model;
3838

39-
/** Returned to FindMachineDialog with the location of the found string
40-
* (or -1 if the string was not found) as well as a flag indicating
39+
/** Return object with the location of the found string (-1 if the string was not found) as well as a flag indicating
4140
* whether the machine wrapped around the end of the document.
4241
*
4342
* @version $Id: FindResult.java 5594 2012-06-21 11:23:40Z rcartwright $
@@ -49,11 +48,11 @@ public class FindResult {
4948
private boolean _allWrapped;
5049

5150
/** Constructor for a FindResult.
52-
* @param document the document where the found instance is located
53-
* @param foundoffset the offset of the instance found
54-
* @param wrapped {@code true} if the search wrapped to the beginning (or end) of the document
55-
* @param allWrapped {@code true} if the search wrapped to the start document
56-
*/
51+
* @param document the document where the found instance is located
52+
* @param foundoffset the offset of the instance found
53+
* @param wrapped {@code true} if the search wrapped to the beginning (or end) of the document
54+
* @param allWrapped {@code true} if the search wrapped to the start document
55+
*/
5756
public FindResult(OpenDefinitionsDocument document, int foundoffset, boolean wrapped, boolean allWrapped) {
5857
_document = document;
5958
_foundoffset = foundoffset;

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -431,9 +431,6 @@ public InteractionsScriptModel loadHistoryAsScript(FileOpenSelector selector)
431431
*/
432432
public List<OpenDefinitionsDocument> getProjectDocuments();
433433

434-
// /** Compiles all open files (all files in project (??) in project mode) */
435-
// public void compileAll() throws IOException;
436-
437434
/** @return true if the model has a project open, false otherwise. */
438435
public boolean isProjectActive();
439436

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
*/
5858
public final class GlobalModelCompileErrorsTest extends GlobalModelTestCase {
5959

60-
public static final Log _log = new Log("GlobalModel.txt", true);
60+
public static final Log _log = new Log("GlobalModel.txt", false);
6161

6262
private static final String FOO_MISSING_VAR_KEYWORD = "class DrScalaTestFoo { yy }";
6363
private static final String BAR_MISSING_DECLARATION_KEYWORD = "class DrScalaTestBar { zz }";

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

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -49,30 +49,33 @@
4949
*/
5050
public final class GlobalModelCompileIOTest extends GlobalModelTestCase {
5151

52+
/* Log _log inherited from GlobalModelTestCase */
53+
5254
/** After creating a new file, saving, and compiling it, this test checks that the new document is in sync after
5355
* compiling and is out of sync after modifying and even saving it.
5456
* Doesn't reset interactions because no interpretations are performed.
5557
*/
5658
public void testClassFileSynchronization() throws BadLocationException, IOException, InterruptedException {
59+
60+
_log.log("+++Starting testClassFileSynchronization");
5761
final OpenDefinitionsDocument doc = setupDocument(BAR_TEXT);
62+
_log.log(doc + " has been set up");
5863
final File file = tempFile();
59-
// System.err.println("Temp source file is " + file.getAbsolutePath());
60-
61-
// System.err.println("testClassFileSynchronization started");
6264

6365
saveFile(doc, new FileSelector(file));
66+
_log.log(doc + " saved as File " + file);
6467

6568
assertTrue("Source file '" + file + "' should exist after save", file.exists());
6669

6770
CompileShouldSucceedListener listener = new CompileShouldSucceedListener();
6871
_log.log("Created CompilerShouldSucceedListener");
6972
_model.addListener(listener);
70-
// System.err.println("Cached class file is " + doc.getCachedClassFile().getAbsolutePath());
73+
_log.log("Cached class file is " + doc.getCachedClassFile().getAbsolutePath());
7174
assertTrue("Class file should not exist before compile", doc.getCachedClassFile() == FileOps.NULL_FILE);
7275
File buildDir = null;
7376
try { buildDir = doc.getSourceRoot(); }
7477
catch(Exception e) { fail("Internal testing error. Test file " + file + " not set up correctly"); }
75-
// System.err.println("Build directory for test is: " + buildDir);
78+
_log.log("Build directory for test is: " + buildDir);
7679
assertTrue("should not be in sync before compile", ! doc.checkIfClassFileInSync());
7780
assertTrue("The state of all open documents should be out of sync", _model.hasOutOfSyncDocuments());
7881
testStartCompile(doc);
@@ -84,12 +87,11 @@ public void testClassFileSynchronization() throws BadLocationException, IOExcept
8487
listener.checkCompileOccurred();
8588

8689
assertTrue("should be in sync after compile", doc.checkIfClassFileInSync());
87-
// System.err.println(_model.getOpenDefinitionsDocuments());
8890
assertTrue("The state of all open documents should be in sync", ! _model.hasOutOfSyncDocuments());
8991

9092
// Make sure .class exists
9193
File compiled = new File(buildDir, "DrScalaTestBar.class");
92-
// System.err.println("compiled class file = " + compiled);
94+
_log.log("compiled class file = " + compiled);
9395

9496
assertTrue("Build directory '" + buildDir + "' exists", buildDir.exists());
9597
assertTrue("Class file '" + compiled + "' should exist after compile", compiled.isFile());
@@ -104,7 +106,7 @@ public void testClassFileSynchronization() throws BadLocationException, IOExcept
104106
saveFile(doc, new FileSelector(file));
105107
assertTrue("should not be in sync after save", ! doc.checkIfClassFileInSync());
106108

107-
// System.err.println("testClassFileSynchronization completed");
109+
_log.log("+++Completing testClassFileSynchronization");
108110
}
109111

110112
/** Ensure that renaming a file makes it out of sync with its class file.
@@ -113,46 +115,47 @@ public void testClassFileSynchronization() throws BadLocationException, IOExcept
113115
public void testClassFileSynchronizationAfterRename() throws BadLocationException, IOException, IllegalStateException,
114116
InterruptedException {
115117

116-
final OpenDefinitionsDocument doc = setupDocument(FOO_TEXT);
117-
final File file = tempFile();
118-
final File file2 = tempFile();
119-
120-
// System.err.println("testClassFileSynchronizationAfterRename started");
118+
_log.log("+++Starting testClassFileSynchronizationAfterRename");
121119

120+
final OpenDefinitionsDocument doc = setupDocument(FOO_TEXT);
121+
final File file = tempFile();
122+
final File file2 = tempFile();
122123
saveFile(doc, new FileSelector(file));
124+
_log.log(doc + " saved as File " + file);
123125

124126
CompileShouldSucceedListener listener = new CompileShouldSucceedListener();
125127
_model.addListener(listener);
126-
// System.err.println("cached class file is " + doc.getCachedClassFile());
128+
_log.log("Cached class file is " + doc.getCachedClassFile());
127129
assertTrue("Class file should not exist before compile", doc.getCachedClassFile() == FileOps.NULL_FILE);
128130
assertTrue("should not be in sync before compile", !doc.checkIfClassFileInSync());
129131
testStartCompile(doc);
130132
listener.waitCompileDone();
133+
_log.log("Compilation finished");
131134
if (_model.getCompilerModel().getNumErrors() > 0) {
132135
fail("compile failed: " + getCompilerErrorString());
133136
}
134137
_model.removeListener(listener);
135138
listener.checkCompileOccurred();
136-
// assertTrue("should be in sync after compile",
137-
// doc.checkIfClassFileInSync());
138139

139-
// Have to wait 1 second so file will have a different timestamp
140+
// Sleep to ensure that renamed document file has a different time stamp than orginal file
140141
Thread.sleep(1000);
141142

142143
// Rename to a different file
143-
saveFileAs(doc, new FileSelector(file2));
144+
saveFileAs(doc, new FileSelector(file2));
144145
assertTrue("should not be in sync after renaming", ! doc.checkIfClassFileInSync());
145146

146-
// System.err.println("testClassFileSynchronizationAfterRename completed");
147+
_log.log("+++Completing testClassFileSynchronizationAfterRename");
147148
}
148149

149150
/** Tests a compile after a file has unexpectedly been moved or delete. */
150151
public void testCompileAfterFileMoved() throws BadLocationException, IOException {
152+
_log.log("+++Starting testCompileAfterFileMoved");
153+
151154
final OpenDefinitionsDocument doc = setupDocument(FOO_TEXT);
152155
final File file = tempFile();
153-
154-
// System.err.println("testCompileAfterFileMoved started");
155156
saveFile(doc, new FileSelector(file));
157+
_log.log(doc + " saved as File " + file);
158+
156159
TestListener listener = new TestListener();
157160
_model.addListener(listener);
158161
file.delete();
@@ -167,12 +170,12 @@ public void run() {
167170
}
168171
});
169172

170-
assertCompileErrorsPresent("compile should succeed", false);
173+
assertCompileErrorsPresent("vacuous compile should succeed", false);
171174

172175
// Make sure .class exists
173176
File compiled = classForScala(file, "DrScalaTestFoo");
174-
assertTrue("Class file shouldn't exist after compile", !compiled.exists());
177+
assertTrue("Class file shouldn't exist after compile", ! compiled.exists());
175178
_model.removeListener(listener);
176-
// System.err.println("testCompileAfterFileMoved completed");
179+
_log.log("+++Completing testCompileAfterFileMoved");
177180
}
178181
}

0 commit comments

Comments
 (0)