Skip to content

Commit c4d4bdb

Browse files
committed
fix: address cross-platform test failures and runtime races
Fix preview highlight retries, Tern initialization recovery, Windows quick-fix paths, Quick Open dismissal, and browser Builder settings. Stabilize fixture cleanup and asynchronous test assertions, add regression coverage, and pin Phoenix Pro fixes. Include the existing Node lockfile metadata update.
1 parent 726e175 commit c4d4bdb

24 files changed

Lines changed: 302 additions & 216 deletions

‎src-node/package-lock.json‎

Lines changed: 15 additions & 117 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/JSUtils/ScopeManager.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1258,7 +1258,9 @@ define(function (require, exports, module) {
12581258
return;
12591259
}
12601260

1261-
if (previousDocument && previousDocument.isDirty) {
1261+
// A failed initial directory lookup leaves this module without a server.
1262+
// There is no cached previous file to update until initialization succeeds.
1263+
if (ternPromise && previousDocument && previousDocument.isDirty) {
12621264
updateTernFile(previousDocument);
12631265
}
12641266

‎src/LiveDevelopment/MultiBrowserImpl/documents/LiveDocument.js‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ define(function (require, exports, module) {
346346

347347
/**
348348
* Highlight all nodes affected by a CSS rule. Should be called by subclass implementations of
349-
* `updateHighlight()`.
349+
* `updateHighlight()`. Failed sends can be retried on subsequent cursor activity.
350350
* @param {string} name The selector whose matched nodes should be highlighted.
351351
*/
352352
LiveDocument.prototype.highlightRule = function (name) {
@@ -357,7 +357,14 @@ define(function (require, exports, module) {
357357
return;
358358
}
359359
this._lastHighlight = highlight;
360-
this.protocol.evaluate("_LD.highlightRule(" + JSON.stringify(name) + ", " + keepSelection + ")");
360+
this.protocol.evaluate("_LD.highlightRule(" + JSON.stringify(name) + ", " + keepSelection + ")")
361+
.fail(() => {
362+
// Attaching an editor can highlight before any preview client connects.
363+
// Do not cache a failed send, or clear a newer selector's pending highlight.
364+
if (this._lastHighlight === highlight) {
365+
this._lastHighlight = null;
366+
}
367+
});
361368
};
362369

363370
/**

‎src/extensions/default/HTMLCodeHints/integ-tests.js‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -169,14 +169,13 @@ define(function (require, exports, module) {
169169
await closeSession();
170170
});
171171

172-
async function _deleteFile(relativeFileName) {
173-
let deleted = false;
174-
FileSystem.getFileForPath(`${testPath}/${relativeFileName}`).unlink(()=>{
175-
deleted = true;
176-
});
177-
await awaitsFor(function () {
178-
return deleted;
179-
}, "extension interface registration notification");
172+
/**
173+
* Remove a fixture, tolerating absence and reporting other filesystem errors.
174+
* @param {string} relativeFileName Fixture name within the test project.
175+
* @return {Promise<void>}
176+
*/
177+
function _deleteFile(relativeFileName) {
178+
return SpecRunnerUtils.deletePathAsync(`${testPath}/${relativeFileName}`, true, FileSystem);
180179
}
181180

182181
async function createAndVerifyFileContents(fileName, firstLineOfContent) {

‎src/languageTools/DefaultProviders.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,8 +1456,16 @@ define(function (require, exports, module) {
14561456
return null; // multi-file / multi-edit / create-rename ops - beyond the one-replace contract
14571457
}
14581458
// Compare as decoded platform paths so URI encoding differences can't break the match.
1459-
var ownUri = this._quickFixClient.uriForPath(filePath);
1460-
if (PathConverters.uriToPath(uri) !== PathConverters.uriToPath(ownUri)) {
1459+
const ownUri = this._quickFixClient.uriForPath(filePath);
1460+
let editPath = PathConverters.uriToPath(uri);
1461+
let ownPath = PathConverters.uriToPath(ownUri);
1462+
if (brackets.platform === "win") {
1463+
// tsserver lowercases drive letters; Phoenix uses the OS-reported uppercase drive.
1464+
// Normalize only the drive, retaining the rest of the path's case distinctions.
1465+
editPath = editPath.replace(/^[a-z]:/i, drive => drive.toUpperCase());
1466+
ownPath = ownPath.replace(/^[a-z]:/i, drive => drive.toUpperCase());
1467+
}
1468+
if (editPath !== ownPath) {
14611469
return null; // edit lands in a different file
14621470
}
14631471
var range = edits[0].range;

‎src/phoenix-builder/builder-connect-dialog.html‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ <h1 class="dialog-title">Phoenix Builder MCP</h1>
44
<ul class="nav nav-tabs no-focus" style="margin-bottom: 0;">
55
<li class="active"><a href="#builder-settings" data-toggle="tab">Settings</a></li>
66
<li><a href="#builder-logs" data-toggle="tab">Console Logs</a></li>
7+
{{#mcpOverrideFile}}
78
<li><a href="#builder-prod" data-toggle="tab">Production</a></li>
9+
{{/mcpOverrideFile}}
810
</ul>
911
</div>
1012
<div class="modal-body tab-content">
@@ -62,6 +64,7 @@ <h1 class="dialog-title">Phoenix Builder MCP</h1>
6264
<span style="opacity: 0.5;">No logs captured yet.</span>
6365
</div>
6466
</div>
67+
{{#mcpOverrideFile}}
6568
<div id="builder-prod" class="tab-pane">
6669
<p class="dialog-message">
6770
MCP is off in production builds. To instrument one, a machine admin drops a dated
@@ -92,6 +95,7 @@ <h1 class="dialog-title">Phoenix Builder MCP</h1>
9295
window shows a red <b>Remote Controlled</b> badge in its status bar.
9396
</p>
9497
</div>
98+
{{/mcpOverrideFile}}
9599
</div>
96100
<div class="modal-footer">
97101
<button class="dialog-button btn primary" data-button-id="ok">Done</button>

‎src/phoenix-builder/main.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,15 +175,17 @@ define(function (require, exports, module) {
175175
$container.scrollTop($container[0].scrollHeight);
176176
}
177177

178+
/** Open Builder settings, including production setup when a native override path is available. */
178179
function _handlePhoenixBuilderConnect() {
179180
let url = localStorage.getItem("phoenixBuilderWsUrl") || DEFAULT_WS_URL,
180181
enabled = localStorage.getItem("phoenixBuilderEnabled") === "true";
181182

182-
// Ready to run, so the Production tab needs nothing looked up elsewhere.
183-
// The path comes from SystemConfigOverride rather than being written out
184-
// again here, so the two cannot drift apart.
185-
const overrideFile = Phoenix.fs.getTauriPlatformPath(SystemConfigOverride.OVERRIDE_FILE_PATH);
186-
const overrideDir = overrideFile.replace(/[/\\][^/\\]+$/, "");
183+
// The production override is a native file. Browser apps only have VFS paths,
184+
// so they expose connection settings and logs without native setup commands.
185+
// Use SystemConfigOverride's path so the instructions stay in sync with its lookup.
186+
const overrideFile = Phoenix.isNativeApp
187+
? Phoenix.fs.getTauriPlatformPath(SystemConfigOverride.OVERRIDE_FILE_PATH) : null;
188+
const overrideDir = overrideFile ? overrideFile.replace(/[/\\][^/\\]+$/, "") : "";
187189
const now = new Date();
188190
const today = now.getFullYear() + "-" +
189191
String(now.getMonth() + 1).padStart(2, "0") + "-" +

‎src/search/QuickOpen.js‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -761,11 +761,9 @@ define(function (require, exports, module) {
761761
initialString = initialString || "";
762762
initialString = prefix + initialString;
763763

764-
// If the underlying search field was already torn down (e.g. the bar
765-
// was closed via focus-loss but the isOpen flag wasn't updated in
766-
// time), fall back to closing cleanly so the caller reopens fresh.
764+
// Closing must go through the bar so isOpen and closePromise stay in sync.
767765
if (!this.searchField || !this.searchField.$input || !this.$searchField || !this.$searchField[0]) {
768-
this.isOpen = false;
766+
this.close();
769767
return;
770768
}
771769

@@ -880,7 +878,10 @@ define(function (require, exports, module) {
880878
resultProvider: this._filterCallback,
881879
formatter: this._resultsFormatterCallback,
882880
onCommit: this._handleItemSelect,
883-
onHighlight: this._handleItemHighlight
881+
onHighlight: this._handleItemHighlight,
882+
// PopUpManager can dismiss results without moving focus (e.g. when no
883+
// document is open). The destroyed search field cannot keep serving the bar.
884+
onDismiss: this.close.bind(this)
884885
});
885886

886887
// Return files that are non-binary, or binary files that have a custom viewer.

‎test/UnitTestSuite.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ define(function (require, exports, module) {
6666
require("spec/KeybindingManager-integ-test");
6767
require("spec/LanguageManager-test");
6868
require("spec/LanguageManager-integ-test");
69+
require("spec/LanguageToolsQuickFix-test");
6970
require("spec/LowLevelFileIO-test");
7071
require("spec/Metrics-test");
7172
require("spec/MultiRangeInlineEditor-test");

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,8 @@ define(function (require, exports, module) {
7979

8080
afterEach(async function () {
8181
await testWindow.closeAllFiles();
82-
expect(DocumentManager.getAllOpenDocuments().length).toBe(0);
82+
await awaitsFor(() => DocumentManager.getAllOpenDocuments().length === 0,
83+
"background readers to release closed documents");
8384
DocumentModule.off(".docTest");
8485
});
8586

@@ -229,7 +230,9 @@ define(function (require, exports, module) {
229230
refresh.finish("external content");
230231
expect(refresh.doc.getText()).toBe("external content");
231232
expect(refresh.doc.isDirty).toBeFalse();
232-
expect(refresh.doc._refCount).toBe(refresh.initialRefCount);
233+
// Refreshing text also starts linting, which temporarily owns a document reference.
234+
await awaitsFor(() => refresh.doc._refCount <= refresh.initialRefCount,
235+
"the refresh and background inspection to release their references");
233236
});
234237

235238
it("should preserve edits made while a disk refresh is pending", async function () {

0 commit comments

Comments
 (0)