Skip to content

Diagnostics of untitled files aren't always cleared (race) #1065

@sungshik

Description

@sungshik

Describe the bug

When "Revert and Close Editor" is used on a non-empty untitled file (e.g., in the UI tests), two publishDiagnostics calls race each other:

  • Call A is triggered by the didChange event (i.e., the "Revert" part of the command);
  • Call B is triggered by the didClose event (i.e., the "Close" part of the command).

Both calls are scheduled on the worker pool. If call A precedes call B, then:

  1. First, diagnostics are set to those of the empty file. In the case of Pico (e.g., in the UI tests), this is a parse error.
  2. Next, diagnostics are cleared.

This is ok. However, if call B precedes call A (which occasionally happens), then the diagnostics are first cleared and next set, so they reappear and are never cleared again. This isn't ok.

To Reproduce (Manual)

Steps:

  1. Open a Rascal terminal, import the language server of Pico (import demo::lang::pico::LanguageServer;), and register it (main()).
  2. Open an untitled file.
  3. Run command "Change Language Mode" and select "Parametric Rascal LSP". An error diagnostic is reported.
  4. Edit the file.
  5. Run command "Revert and Close Editor". Sometimes the error diagnostic is removed; sometimes it isn't.

Steps 2-5 might need to be repeated a few times because of scheduling.

To Reproduce (UI Tests)

To reproduce in UI tests, the following could be added to afterEach in dsl.test.ts to check if all error diagnostics have been cleared:

afterEach(async function () {
if (this.test?.title) {
await ide.screenshot(`DSL-${errorRecovery}-`+ this.test?.title);
}
await ide.cleanup();
await fs.writeFile(TestWorkspace.picoFile, picoFileBackup);
});

// Add after line 102:
const bottomBar = bench.getBottomBar();
const problemsView = await bottomBar.openProblemsView();
const allVisibleMarkers = await problemsView.getAllVisibleMarkers(MarkerType.Error);
expect(allVisibleMarkers.length, "Not all error diagnostics have been cleared").to.equal(0);

With this addition, the UI tests most of the time fail:

  1) DSL+recovery
       "after each" hook for "has syntax highlighting in documents without extension":

      Not all error diagnostics have been cleared
      + expected - actual

      -1
      +0

(In general, the addition isn't perfect yet, as it checks only the "visible" diagnostics, so if scrolling down past a large number of warnings is needed to find the first error, then that error is missed and the check mistakenly succeeds. For all current tests, though, I think this works fine.)

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions