[CLOSED] Fix Find/Replace unit tests and clean up event handling for animations

[core-ai-bot]

Issue by njx
Saturday Sep 14, 2013 at 00:21 GMT
Originally opened as adobe/brackets#5213

---

For #5094. See my last comment in that bug for more background on why the unit tests were only intermittently failing.

It might be worth reviewing each commit separately since they're all semantically independent (except for the last one, which requires the first three), although I think they're all worth fixing in the course of dealing with the unit tests. Here's a summary of each one:

1. 9d8a24e: In the various places where we listen for webkitAnimationEnd, we weren't checking that the target of the event was actually the item we're listening to (because I didn't know that that event bubbles). This commit fixes that. (It unfortunately means we can't just use $.one() anymore because we only want to disconnect the event handler when we get the right target.) By itself, this would make it so that the existing Find/Replace tests would consistently fail (as they should have been, see (4) below), instead of only intermittently failing.
2. c0b6510: This just adds a private parameter to the handler for FILE_CLOSE so that we can force a document to close even if it has dirty changes. This is useful for unit tests in a test window where we want to be able to manipulate a document but discard any changes we make before the next test.
3. 86dc22c: This makes it so that the Find bar deliberately closes whenever the user switches documents. Currently, it usually closes, as a side-effect of focus changing when the document switches, but it doesn't close when the last document is closed. I mostly did this to support the changes to unit tests in (4), but I think it makes sense from a user point of view anyway.
4. 450642b: This is the actual fix to the unit tests. Originally they ran in the test-runner window, but in that environment they don't get the real Brackets CSS. This was a problem because the Brackets CSS creates an animation for closing the modal bar, and the modal bar waits until that animation is complete before removing itself from the DOM. Because of the issues fixed by (1), however, we didn't always see this failure due to other transitions in the Bootstrap CSS in the test window. I could have hacked this somehow via a mock, but since the Find/Replace tests are really more UI integration tests than model tests, it made sense to me to have the existing tests run in a real Brackets test window anyway.

Note that the new tests basically create a single window, and then repeatedly create new untitled documents to run the tests in. That's why I wanted to make the fixes in (2) and (3), and also changed one of the tests (which was assuming the file was color-coded). For that last issue, I could have just saved a JS file and tested against that, but it actually seemed cleaner to me to not have the tests depend on a particular color-coding.

---

njx included the following code: https://github.com/adobe/brackets/pull/5213/commits

[core-ai-bot]

Comment by njx
Saturday Sep 14, 2013 at 00:27 GMT

---

Oops, I left some comments on the last commit instead of in the Files tab.

[core-ai-bot]

Comment by redmunds
Monday Sep 16, 2013 at 22:35 GMT

---

Done with review.

[core-ai-bot]

Comment by njx
Tuesday Sep 17, 2013 at 05:27 GMT

---

Ready for re-review.

[core-ai-bot]

Comment by redmunds
Tuesday Sep 17, 2013 at 14:57 GMT

---

Merging.