[CLOSED] Smallish cleanups to docs & unit tests; improve a few tests

[core-ai-bot]

Issue by peterflynn
Friday Apr 12, 2013 at 21:40 GMT
Originally opened as adobe/brackets#3429

---

• SpecRunnerUtils:
  • Improve docs
  • Add assertion in case a mock Document is created without an Editor but then used in a way that would auto-create an Editor (which won't get cleaned up)
  • In DocumentManager, add assertion in _destroyEditorIfUnneeded() since its name makes it easy to call with the wrong type (including via SRU.destroyMockEditor())
  • Use waitsForDone() more
• LanguageManager:
  • Minor docs cleanups (incorrect usage of "i.e.", missing
    docs for new fileNames option, etc.)
  • Remove a duplicated LanguageManager test (due to f67cfcd)
  • Move filename->language mapping tests from Editor-test to LanguageManager-test
• InstallExtensionDialog: Add unit tests for install dialog cases where installer comes back with a result after clicking Cancel but before Cancel times out, or comes back vary late after timeout once user has opted to close the dialog
• StringMatch: Add a few more tests to ensure tricky cases don't break in the opposite direction as before (including the test suggested by Kevin in [CLOSED] Jenkins updates #3417)
• DocumentManager: use CollectionUtils.forEach() in notifyPathNameChanged() (removing early-exit optimization that's probably unneeded; can reintroduce when CollectionUtils.some() lands)
• FileIndexManager: add another [CLOSED] LESS fails with an empty declaration #330 testcase as suggested by Glenn in [CLOSED] Fix #2645: [FRA, JPN]: The word "Loading..." is not localized. #3064
• HTMLCodeHints: rename test suite & remove unused var
• CSSUtils: remove unused vars
• Dialogs: tiny docs improvement

---

peterflynn included the following code: https://github.com/adobe/brackets/pull/3429/commits

[core-ai-bot]

Comment by njx
Friday May 10, 2013 at 01:56 GMT

---

Open for committers to review.

[core-ai-bot]

Comment by TomMalbran
Tuesday Jul 23, 2013 at 02:52 GMT

---

I just reviewed this request, so I guess I assigned it to myself.

It all looks great, just some minor comments.

[core-ai-bot]

Comment by peterflynn
Tuesday Jul 23, 2013 at 04:56 GMT

---

@RaymondLim All merged up with master now, whenever you want to review. (Or@TomMalbran since it looks like you started looking at this, would you want to do a full review instead?)

Not important for the current sprint though, so later this week is fine either way.

[core-ai-bot]

Comment by TomMalbran
Tuesday Jul 23, 2013 at 05:15 GMT

---

@peterflynn I did a full review and tested before your last commit. All new tests where passing and everything seems to have been working fine. I was even about to assign me as this was an Open request when I saw that@RaymondLim was assigned to it.

The new changes seem good, but instead of re-indenting, what about creating a new variable instead of replacing the old ones? With a new var statement, the strings can be aligned, and in some cases it makes it easier to read the tested code text.

[core-ai-bot]

Comment by peterflynn
Tuesday Jul 30, 2013 at 03:41 GMT

---

@TomMalbran I think this addresses all the other comments. I've reassigned to you. We haven't tagged Sprint 28, so no merging yet -- but hopefully tomorrow morning we'll be good to go. Feel free to hit merge whenever you want after that. Let me know if you need any other changes here.

[core-ai-bot]

Comment by TomMalbran
Tuesday Jul 30, 2013 at 04:24 GMT

---

@peterflynn Everything looks fine. I found a small Nit, but on the previous code, so might fix it later. Will rerun the tests later and merge when we are in Sprint 28.

[core-ai-bot]

Comment by peterflynn
Tuesday Jul 30, 2013 at 17:12 GMT

---

@TomMalbran Cool, ok -- we're open for merges now.

[core-ai-bot]

Comment by TomMalbran
Tuesday Jul 30, 2013 at 20:15 GMT

---

@peterflynn I found some more nits on the JSDoc types. Is all small ones, but since we are fixing the JSDocs here, we should fix these too. I can fix these and merge too. Everything else is fine, and tests are passing.

[core-ai-bot]

Comment by peterflynn
Tuesday Jul 30, 2013 at 23:34 GMT

---

@TomMalbran Pushed the other cleanups

[core-ai-bot]

Comment by TomMalbran
Tuesday Jul 30, 2013 at 23:49 GMT

---

Thanks for the cleanups. Merging now.