Unit tests for Find #2324

Merged
merged 4 commits into from Dec 11, 2012

Projects

None yet

2 participants

@peterflynn
Member

New suite of unit tests for Find (but not Replace, yet).

Also cleans up a few other test suites to use KeyEvent consts instead of magic numbers, and to remove unused dependencies.

@njx or @gruehle since you're currently mucking around Find code?

peterflynn added some commits Dec 7, 2012
@peterflynn peterflynn Initial set of unit tests for Find 1abb001
@peterflynn peterflynn Merge remote-tracking branch 'origin/master' into pflynn/find-unit-tests
* origin/master:
  Code cleanup.
  Add no-focus to jslint and search results
  Remove empty style rule
  More improvements in Find.
  Add .no-focus style and implementation.
  Refine find logic with pre-selected query string.
4c9bb22
@peterflynn peterflynn Add a few more unit tests for Find. Update unit-test harness files to
include this suite.

Update a few other unit tests to use KeyEvent constants, and remove unused
dependencies. (All those suites still pass too).
1328419
@gruehle gruehle commented on an outdated diff Dec 10, 2012
test/SpecRunner.html
@@ -36,6 +36,9 @@
<!-- Pre-load third party scripts that cannot be async loaded. -->
<script src="../src/thirdparty/jquery-1.7.min.js"></script>
<script src="../src/thirdparty/CodeMirror2/lib/codemirror.js"></script>
+ <script src="../src/thirdparty/CodeMirror2/lib/util/dialog.js"></script>
+ <script src="../src/thirdparty/CodeMirror2/lib/util/searchcursor.js"></script>
+ <script src="../src/thirdparty/CodeMirror2/lib/util/search.js"></script>
gruehle
gruehle Dec 10, 2012 Member

search.js is not needed. All the functionality from this file has been superseded by FindReplace.js

(pull request #2314 removes search.js from index.html)

@gruehle gruehle was assigned Dec 10, 2012
@gruehle gruehle commented on the diff Dec 10, 2012
test/spec/FindReplace-test.js
+
+ CommandManager.execute(Commands.EDIT_FIND);
+
+ // This should be interpreted as a non-RegExp search, which actually does have a result thanks to "modules/Bar"
+ enterSearchText("/Ba");
+ expectSelection({start: {line: LINE_FIRST_REQUIRE + 1, ch: 30}, end: {line: LINE_FIRST_REQUIRE + 1, ch: 33}});
+ });
+
+ it("shouldn't choke on empty regexp", function () {
+ myEditor.setCursorPos(0, 0);
+
+ CommandManager.execute(Commands.EDIT_FIND);
+
+ enterSearchText("//");
+ expectCursorAt({line: 0, ch: 0}); // no change
+ });
gruehle
gruehle Dec 10, 2012 Member

Do you think there should be tests for an invalid regexp? We could make sure the .alert-message div is present and that the code doesn't choke.

Member
gruehle commented Dec 10, 2012

Initial review complete.

Member

@gruehle: changes pushed

Member
gruehle commented Dec 11, 2012

Looks good.

@gruehle gruehle merged commit ebc18c9 into master Dec 11, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment