Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Swap out the user prefs when running tests. #7163

Merged
redmunds merged 5 commits into from
Mar 20, 2014
Merged

Swap out the user prefs when running tests. #7163

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
abb81d1
Swap out the user prefs when running tests.
dangoor Mar 11, 2014
f82ca6a
Merge branch 'master' into dangoor/7054-test-no-break-pref
dangoor Mar 13, 2014
8d6df0e
Merge branch 'master' into dangoor/7054-test-no-break-pref
dangoor Mar 18, 2014
f675c19
Extract the creation of the preferences system to a new module.
dangoor Mar 19, 2014
008a64e
Uses the TestPreferencesImpl when running tests.
dangoor Mar 19, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions test/SpecRunner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ define(function (require, exports, module) {
NodeDomain = require("utils/NodeDomain"),
BootstrapReporterView = require("test/BootstrapReporterView").BootstrapReporterView,
ColorUtils = require("utils/ColorUtils"),
PreferencesManager = require("preferences/PreferencesManager"),
PreferencesBase = require("preferences/PreferencesBase"),
NativeApp = require("utils/NativeApp");

// Load modules that self-register and just need to get included in the test-runner window
Expand Down Expand Up @@ -133,6 +135,18 @@ define(function (require, exports, module) {
}

function _documentReadyHandler() {
var pm = PreferencesManager._manager,
sm = PreferencesManager.stateManager;
pm.removeScope("user");
Copy link
Copy Markdown
Contributor

@redmunds redmunds Mar 11, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

@dangoor dangoor Mar 13, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@redmunds redmunds Mar 13, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

@redmunds redmunds Mar 13, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@dangoor dangoor Mar 14, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@TomMalbran TomMalbran Mar 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

pm.addScope("user", new PreferencesBase.MemoryStorage(), {
before: "default"
});

sm.removeScope("user");
sm.addScope("user", new PreferencesBase.MemoryStorage(), {
before: "default"
});

if (brackets.app.showDeveloperTools) {
$("#show-dev-tools").click(function () {
brackets.app.showDeveloperTools();
Expand Down
⚡