Web: integration test failures in workspace events

[bpasero]

Only failing in Web and only running out of sources it seems:

 2 failing
  1) vscode API - workspace events: onDidOpenTextDocument, onDidChangeTextDocument, onDidSaveTextDocument:
      AssertionError [ERR_ASSERTION] [ERR_ASSERTION]: /ijnhhxi-NEW <-> /bombyiu
      + expected - actual
      -false
      +true
      at assertEqualPath (extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts:284:10)
      at extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts:302:30
      at extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts:323:36
      at Array.forEach (<anonymous>)
      at Context.<anonymous> (extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts:323:18)
      at processTicksAndRejections (internal/process/task_queues.js:89:5)
  2) vscode API - workspace events: onDidSaveTextDocument fires even for non dirty file when saved:
      AssertionError [ERR_ASSERTION] [ERR_ASSERTION]: /eyglyouflp <-> /ytgwtrbtfq
      + expected - actual
      -false
      +true
      at assertEqualPath (extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts:284:10)
      at extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts:335:30
      at extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts:344:32
      at Array.forEach (<anonymous>)
      at Context.<anonymous> (extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts:344:18)
      at processTicksAndRejections (internal/process/task_queues.js:89:5)

[jrieken]

vscode/src/vs/workbench/api/browser/mainThreadDocuments.ts

Line 149 in 8025723

this._proxy.$acceptModelSaved(e.model.resource);

⬆️ that's where the onDidSaveTextDocument-event is driven from. So, either a problem with the web-logic or because of the setup. E.g auto-save in web vs manual save in desktop?

[bpasero]

@jrieken oh yeah, auto save is on by default in web, good catch! could that be the reason?

[jrieken]

idk - I would still expect a save event to be fired when calling save (as done here). Tho, it might be the wrong command 🤷. From the failure logs it seems that something gets saved but not what we are expecting...

[bpasero]

@jrieken this seems to be an issue with how tests are setup. If I only run the workspace tests I see none failing, via suite.only:

vscode/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts

Line 13 in 75e134a

suite('vscode API - workspace', () => {

A listener not cleaned up?

[jrieken]

A listener not cleaned up?

Only in the web, unlikely. Maybe missing cleanup because of a failure... So, the second could be a false-alarm because the first failed?

[bpasero]

This is indeed caused by auto save being enabled by default in web, changing this line fixes the test failure:

vscode/src/vs/workbench/contrib/files/browser/files.contribution.ts

Line 307 in c828a53

'default': platform.isWeb ? AutoSaveConfiguration.AFTER_DELAY : AutoSaveConfiguration.OFF,

So in theory, a test maybe leaves a model dirty, then continues to run other tests and then after 1s that previous model is being auto saved.

I see two options:

• we simply disable auto save when tests run
• the owner of these tests checks if possibly a model is not getting disposed at the end of the test and thus has a chance to get auto saved

[bpasero]

It would be easier if somehow the name of the test could be part of the models that are being created so that we know from which test the save kicks in.

[jrieken]

The weird thing is that the first failing tests already calls saveAll. So, either that's not working or another leaked listener modifies files based on those event. The -NEW suffix is a hint, there isn't so many tests using that but nothing obvious

[bpasero]

I added a new command _workbench.revertAllDirty as a replacement for saveAll and added it to the two tests that rely on the onDidSaveTextDocument event.

Bottom line: saveAll only goes over the opened dirty editors and saves them, but does not save dirty working copies that do not have an associated editor (in this case we had dirty models from bulk edits in the background not being opened). I did not want to go as far as to change saveAll to work on hidden working copies so I found this the safer fix.