Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integration test failure: openTextDocument, untitled closes on save #157897

Closed
alexr00 opened this issue Aug 11, 2022 · 5 comments
Closed

Integration test failure: openTextDocument, untitled closes on save #157897

alexr00 opened this issue Aug 11, 2022 · 5 comments

Comments

@alexr00
Copy link
Member

alexr00 commented Aug 11, 2022

Failed in https://dev.azure.com/vscode/VSCode/_build/results?buildId=70964&view=logs&j=a8a6689c-5a84-59c0-482a-7dd9f21eb2d8&t=1fa6fb8d-7bb3-5acd-5778-bf99aac7f637

MainThreadFileSystem#$watch(): failed to stat a resource for file watching (extension: vscode.vscode-api-tests, path: watcherTest:/somePath/folder, recursive: true, session: 0.4519563871799428): EntryNotFound (FileSystemError): watcherTest:/somePath/folder
    ✔ createFileSystemWatcher
  206 passing (16s)
  63 pending
  1 failing
  1) vscode API - workspace
       openTextDocument, untitled closes on save:

      AssertionError [ERR_ASSERTION] [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert.ok(closed === doc)

      + expected - actual

      -false
      +true

@bpasero
Copy link
Member

bpasero commented Aug 11, 2022

Again, could be a regression from dced70b, looks like this test failure is relatively new.

One actual change from dced70b is that openEditor will not reach group.openEditor(input) without yielding once, due to the new async await here:

const resolvedEditor = await this.editorResolverService.resolveEditor(editor, preferredGroup);

Since we no longer disable the editor override here:

override: DEFAULT_EDITOR_ASSOCIATION.id

If there is any code in the renderer that relied on the previous behaviour, maybe that change made the tests more flaky now?

@bpasero
Copy link
Member

bpasero commented Aug 11, 2022

My theory is that possibly closed is actually undefined and so pushing a change to assert it is there.

const d0 = vscode.workspace.onDidCloseTextDocument(e => closed = e);

bpasero added a commit that referenced this issue Aug 11, 2022
sapiderman pushed a commit to sapiderman/vscode that referenced this issue Aug 11, 2022
@bpasero bpasero modified the milestones: August 2022, September 2022 Aug 16, 2022
@bpasero
Copy link
Member

bpasero commented Aug 18, 2022

This failed again for web in: https://dev.azure.com/monacotools/Monaco/_build/results?buildId=182276&view=logs&j=a8b516b9-5c25-5ad9-505a-b03d38bc1dad&t=b2e05f10-ada9-503f-2369-d13399289d47&l=352

Given my change is in, it looks like literally even though we have closed document from the event, it is not the same as doc:

return doc.save().then((didSave: boolean) => {
assert.strictEqual(didSave, true, `FAILED to save${doc.uri.toString()}`);
assert.ok(closed);
assert.ok(closed === doc);
assert.ok(!doc.isDirty);
assert.ok(fs.existsSync(path));
d0.dispose();
fs.unlinkSync(join(vscode.workspace.rootPath || '', './newfile.txt'));
});

I am pushing a change to compare the URIs of the two and see what they are.

@bpasero
Copy link
Member

bpasero commented Aug 28, 2022

I see no failure since, let's close until we hit this again.

@bpasero bpasero closed this as completed Aug 28, 2022
@bpasero
Copy link
Member

bpasero commented Sep 2, 2022

Failed again in:

https://dev.azure.com/monacotools/Monaco/_build/results?buildId=184110&view=logs&j=a8b516b9-5c25-5ad9-505a-b03d38bc1dad&t=b2e05f10-ada9-503f-2369-d13399289d47&l=349

 AssertionError [ERR_ASSERTION] [ERR_ASSERTION]: closed.uri = fake-fs:/f7c6a90cfe9c7c87 but doc.uri = untitled:/__w/1/s/extensions/vscode-api-tests/testWorkspace/newfile.txt

This looks as if a document from a previous test run appears in our event listener here:

const d0 = vscode.workspace.onDidCloseTextDocument(e => closed = e);

I guess we cannot guarantee that the event that comes in is for the closed editor, there maybe multiple. So an idea to fix this is to keep closed documents in an array for the test and try to find the one we expect: #159888

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants