Skip to content

Commit a50162f

Browse files
committed
fix(project): keep a newer project's preferences when an older load finishes last
Opening a project reloads its preferences file in the background without waiting. When two projects opened close together, the earlier project's reload could finish last and point the project preferences back at the project that had just closed, leaving settings like spaceUnits at their defaults. This made the PreferencesManager integration tests flaky on CI. The reload now stops if the project root changed while it read the files. A regression test holds one project's reads while the next project opens, and fails without the fix.
1 parent e52646a commit a50162f

2 files changed

Lines changed: 60 additions & 1 deletion

File tree

‎src/project/ProjectManager.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,6 +1117,13 @@ define(function (require, exports, module) {
11171117
bracketsPrefFile = root.fullPath + SETTINGS_FILENAME_BRACKETS;
11181118
const statusPhoenix = await _validateProjectPreferencesFile(phoenixPrefFile);
11191119
const statusBrackets = await _validateProjectPreferencesFile(bracketsPrefFile);
1120+
// Another project opened while the files were read. Its own reload
1121+
// applies; finishing this one would point the project preferences
1122+
// back at the project that was just closed.
1123+
const currentRoot = getProjectRoot();
1124+
if (!currentRoot || currentRoot.fullPath !== root.fullPath) {
1125+
return;
1126+
}
11201127
let prefFileToUse = phoenixPrefFile;
11211128
if(statusPhoenix === PREF_NO_FILE && statusBrackets !== PREF_NO_FILE) {
11221129
prefFileToUse = bracketsPrefFile;

‎test/spec/PreferencesManager-integ-test.js‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
*
2020
*/
2121

22-
/*global describe, it, expect, beforeEach, beforeAll, afterAll, awaitsFor, awaitsForDone */
22+
/*global describe, it, expect, beforeEach, beforeAll, afterAll, awaitsFor, awaitsForDone, spyOn */
2323

2424
define(function (require, exports, module) {
2525

@@ -84,6 +84,58 @@ define(function (require, exports, module) {
8484
await _verifySinglePreference(".brackets.json", 6);
8585
}, 100000);
8686

87+
it("should keep a newer project's preferences when an older project's load finishes last", async function () {
88+
const FileUtils = testWindow.brackets.test.FileUtils;
89+
const projectWithoutSettings = SpecRunnerUtils.getTestPath("/spec/WorkingSetView-test-files");
90+
const realReadAsText = FileUtils.readAsText;
91+
let releaseStaleReads;
92+
const staleReadsReleased = new Promise(function (resolve) {
93+
releaseStaleReads = resolve;
94+
});
95+
let heldReads = 0, settledReads = 0;
96+
// Hold the settings-free project's preference reads until the next
97+
// project has loaded, so its reload finishes last.
98+
spyOn(FileUtils, "readAsText").and.callFake(function (file, ...args) {
99+
const result = realReadAsText.call(FileUtils, file, ...args);
100+
const name = file.fullPath.split("/").pop();
101+
if (!file.fullPath.startsWith(projectWithoutSettings + "/") ||
102+
(name !== ".phcode.json" && name !== ".brackets.json")) {
103+
return result;
104+
}
105+
heldReads++;
106+
const held = new testWindow.$.Deferred();
107+
staleReadsReleased.then(function () {
108+
result.always(function () {
109+
settledReads++;
110+
}).done(held.resolve).fail(held.reject);
111+
});
112+
return held.promise();
113+
});
114+
spyOn(PreferencesManager, "_setProjectSettingsFile").and.callThrough();
115+
try {
116+
// Start elsewhere, so opening the settings-free project is a real switch.
117+
await SpecRunnerUtils.loadProjectInTestWindow(testPath);
118+
await SpecRunnerUtils.loadProjectInTestWindow(projectWithoutSettings);
119+
await SpecRunnerUtils.loadProjectInTestWindow(testPathBracketsPrefsOnly);
120+
await awaitsForDone(SpecRunnerUtils.openProjectFiles(".brackets.json"));
121+
await awaitsFor(()=>{
122+
return PreferencesManager.get("spaceUnits") === 6;
123+
}, "space units to be 6 from the newer project", 10000);
124+
expect(heldReads).toBeGreaterThan(0);
125+
} finally {
126+
releaseStaleReads();
127+
}
128+
// Every held read settles, and whatever the older reload does next
129+
// runs before the next check.
130+
await awaitsFor(()=>{
131+
return settledReads === heldReads;
132+
}, "the older project's reads to finish", 10000);
133+
134+
const lastSettingsFile = PreferencesManager._setProjectSettingsFile.calls.mostRecent().args[0];
135+
expect(lastSettingsFile.startsWith(testPathBracketsPrefsOnly + "/")).toBeTrue();
136+
expect(PreferencesManager.get("spaceUnits")).toBe(6);
137+
}, 30000);
138+
87139
it("should .phcode.json take precedence over .brackets.json", async function () {
88140
await SpecRunnerUtils.loadProjectInTestWindow(testPathBothPrefs);
89141
await awaitsForDone(SpecRunnerUtils.openProjectFiles(".phcode.json"));

0 commit comments

Comments
 (0)