Swap out the user prefs when running tests.

[dangoor]

Fix for #7054. Note that the exception that appears in the console when running the tests is separate: #7161

This should prevent tests that run in the specrunner window from clobbering
user-level prefs.

cc @redmunds

[redmunds]

Should project prefs also be removed? They should not affect tests and we don't want them getting updated either.

[dangoor]

The SpecRunner doesn't have a project open by default. I was thinking that if a test explicitly opens a project, then it would be okay (and possibly even desirable) to load the project prefs.

[redmunds]

SpecRunner does not open a project, but every Integration test that opens a test window opens a project by default (Brackets requires one).

[redmunds]

Also, what about view state? I found this case that breaks tests and alters view state prefs:

Recipe

1. Open Brackets, create a temporary file, and save it to disk
2. Double-click on temporary file so it's current file & in Working Set
3. Close Brackets
4. Delete temporary file (outside of Brackets)
5. Restart Brackets
6. You will receive warning that "temporary file no longer exists" (which is expected), click OK
7. Debug > Run Tests, open Integration tab, click "All"

Results:
You get the same warning that "temporary file doesn't exist" for every test that opens a new window, so it seems like tests are loading view state prefs from disk and not memory, but I wonder if they should load them at all.

I walked away from my computer, so I had lots of modal dialogs waiting to be clicked, so a lot of tests failed due to timeout. If you sat there and quickly responded, you might be able to get tests to pass, but that's just silly :)

Continue:
8. Shutdown & restart Brackets

Results:
The good news is that all of my user prefs seemed to be preserved. But, I got the "temporary file doesn't exist" dialog again, so view state prefs did not seem to get saved.

[dangoor]

Good catch. I don't generally like production code to behave specially for tests, but it may be required in this case, given how early in Brackets' startup viewstate and preferences are used.

[TomMalbran]

Swapping the View State should fix issue #7215 since the test assumes that you start on the default font size, but it doesn't if you change the font-size in Brackets. Even if we fix that without this PR, we would need to restore the original font-size after the tests are done, so I think that we shouldn't even use the user's view state.

[redmunds]

I was able to hit the problem again in this branch on Win7. Here's what I did:

1. Right before opening Spec Runner window:
   • change projects
   • change state of open/closed folders in project tree
   • remove file from working set
   • double-click on a file to open and add it to working set
2. Debug > Run tests, All/All
3. Tests caused Brackets to hang (see note below), so Open Task Manager and End Process for Brackets
4. Restart Brackets

Results
When Brackets re-opened, it remembered the new project and the project tree state, but it did not remember which file was open in the working set (it showed the one I closed and not the one I opened).

So, it seems that you need to force write all of the pending data changes to disk before removing the "user" prefs and state?

Note: I recently got a new lenovo laptop with SSD, and it usually hangs when trying to run All/All tests.

[dangoor]

@redmunds With those steps to reproduce, that sounds like an issue with viewstate generally and not related to the SpecRunner window. Basically, if anything caused Brackets to crash and you had to end the process, you would likely have had the same results. That's probably worth a different bug, I think.

What this change is intended to fix is if any test modifies the preferences or view state it shouldn't change the real prefs or view state of the person running the test.

[redmunds]

@dangoor Yes, I guess crash Brackets is a different case, but I'm certain I have also hit that for case of failing test(s). What about this comment?

[dangoor]

@redmunds It's possible that I've missed something, but I think with this branch the tests should not be interacting with your own prefs at all.

[dangoor]

Updated to reliably replace the preferences systems used in testing with memory-backed ones that never touch the user's file. It works by reconfiguring RequireJS to change which module is used for creating the PreferenceSystem instances.

[redmunds]

FYI, I opened a separate issue for The default for linting should be off.

[redmunds]

All of the bugs related to tests mentioned so far seem to be fixed!

But I'm having problems running Integration tests on Win7 (Mac 10.8 passes). I switched back to master and they passed except for the known issues (being fixed in this PR). Since those issues are related to prefs, I tried the following:

1. Shutdown Brackets
2. Delete brackets.json and state.json files
3. Make sure git status is clean
4. Start Brackets, Debug > Run Tests
5. Select Integration tab, run "All"

The Results are close to what I am seeing in your branch

Results on master branch with no prefs:

Results in your branch (which seems to have less failures than previous runs):

Note: The Integration tests mostly pass if I run 1 set of tests at a time. I'm not sure how this fits into the formula.

[RaymondLim]

@redmunds I'm getting all "Live Development" and "Inspector" tests (10 in total) fail in master on my win7, but only 8 in "Live Development" and two in MultiRangeInlineEditor failing in this pull request branch. If I re-run all Integration tests again, then all of them pass in this branch.

[redmunds]

@RaymondLim It's weird that tests (mostly?) pass if I run 1 set at a time. The problem is when I run Integration/All. Does that work for you?

[RaymondLim]

Yes, I'm running Integration/All.

[redmunds]

OK, I guess I need to run each set one at a time. All test pass on Win7 except for 1 EditorOptionHandlers unit test:

"should also wrap long lines in inline editor by default"

Error: Expected 30 to be less than 30.
    at new jasmine.ExpectationResult (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:102:32)
    at null.toBeLessThan (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1194:29)
    at null.<anonymous> (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/spec/EditorOptionHandlers-test.js:101:45)
    at jasmine.Block.execute (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1024:15)
    at jasmine.Queue.next_ (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1842:31)
    at onComplete (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1838:18)
    at jasmine.WaitsForBlock.execute (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:2322:5)
    at file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:2336:12

[dangoor]

I just ran all integration tests on my branch (on Mac) and they all passed. I have sporadically seen failures on master (or other branches) for the Live Development tests. I also just ran EditorOptionHandlers test alone and it passed.

@redmunds just to double check: you do have caching turned off and the devtools open when running the test, right? That EditorOptionHandlers test would fail on master currently if your font size is increased.

With the change introduced in this PR, the only new failure I might expect is one where some piece of code expects userPrefFile to be something other than null, but there's nothing in the code that does that.

[redmunds]

@dangoor I haven't specifically turned off caching in devtools, but devtools is not running when I run unit tests. I manually removed all extensions (since I wasn't sure if "Reload Without Extensions" extended to test windows). This test passes for me on Mac 10.8, but fails every time on Win7.

[dangoor]

@redmunds from what I've seen, if you don't explicitly set caching off and have dev tools open when you run the tests, the specrunner will run whatever test code was there when you launched Brackets. The only way I reliably see tests running the code that is actually sitting on disk at that moment is to have the dev tools open with caching disabled.

[dangoor]

I just ran all of the integration tests on Windows (7) and they all passed for me.

[redmunds]

I seem to be the only one who's having problems with unit tests, and this fixes some significant problems running tests, so I'm going to merge it.

[redmunds]

@dangoor

if you don't explicitly set caching off and have dev tools open when you run the tests, the specrunner will run whatever test code was there when you launched Brackets

Yes, I always shutdown/restart to be safe.

I seem to be the only one who's having problems with unit tests...

I submitted a fix for this in pull request #7291.