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

Duplicated launch configs #12153

Closed
marcdumais-work opened this issue Feb 6, 2023 · 3 comments
Closed

Duplicated launch configs #12153

marcdumais-work opened this issue Feb 6, 2023 · 3 comments
Assignees
Labels
debug issues that related to debug functionality

Comments

@marcdumais-work
Copy link
Contributor

marcdumais-work commented Feb 6, 2023

Bug Description:

I've been working to improve the browser test suite and when I updated my baseline to include the content of the most recent master branch, over 100 tests from launch-preferences.spec.js suddenly started consistently failing.

This test file is a bit obscure, but I think the fundamental reason for the failures is that we end-up with duplicated launch configurations, whenever the preferences machinery evaluates them. I have traced the issue to PR #12126, which makes a change in PreferenceProvider.

Note: the browser test suite has been very flaky lately, so it's no surprise its failures ends-up being not noticed during PR reviews. Additionally, the new failures related to the launch preferences generate so many error traces that I think the GitHub runner gets overwhelmed and ends-up not showing the detailed errors at the end of execution, which makes it very hard to notice that there are additional new failures (vs the usual ones).

To run the failing test file locally and get access to the full output log:

  1. build the Theia repo from sources: yarn && yarn browser build && yarn download:plugins
  2. cd examples/browser
  3. yarn -s rebuild && npx theia test . --plugins=local-dir:../../plugins --test-spec=../api-tests/src/launch-preferences.spec.js

update: If the test failure traces are still not present in the output, try running in non-headless mode:
yarn -s rebuild && npx theia test . --plugins=local-dir:../../plugins --test-inspect --test-spec=../api-tests/src/launch-preferences.spec.js

Steps to Reproduce:

  1. Build the Theia repo from sources
  2. Use the example Theia application to open folder examples/browser as your workspace
  3. Switch to the debug view.
  4. Trigger the launch drop-down and select Add Configuration and select a type of launch (e.g.: "Node.js: Launch Program")
  5. Trigger the launch drop-down again and confirm that two launches are listed, under the name of the new added launch (e.g. "Launch Program"

Alternatively, open the Theia repo root folder as a workspace and confirm that all launch are duplicated (some maybe more than one time).

The same duplication happens if one adds a launch to the settings.json file.

See this video capture:
duplicated launches

Additional Information

Not sure if it might help, but while debugging I have added some printouts to preferences-provider.ts and captured the following traces, when adding a launch configuration, using the reproduce steps above:

static merge(source: JSONValue | undefined, target: JSONValue): JSONValue {
        const origSource = source;
        if (source === undefined || !JSONExt.isObject(source)) {
            return JSONExt.deepCopy(target);
        }
        if (JSONExt.isPrimitive(target)) {
            return {};
        }
        for (const key of Object.keys(target)) {
            const value = (target as any)[key];
            if (key in source) {
                if (JSONExt.isObject(source[key]) && JSONExt.isObject(value)) {
                    this.merge(source[key], value);
                    continue;
                } else if (JSONExt.isArray(source[key]) && JSONExt.isArray(value)) {
                    console.log('\n\n');
                    console.log('%%%% merge: source: ' + JSON.stringify(origSource));
                    console.log('%%%% merge: target: ' + JSON.stringify(target));
                    console.log('********************** source[key] and value are both arrays');
                    console.log('********************** key          :       ' + key);
                    console.log('********************** value target[key]    ' + JSON.stringify(value));
                    console.log('********************** source b4 update:    ' + JSON.stringify(source));
                    source[key] = [...JSONExt.deepCopy(source[key] as any), ...JSONExt.deepCopy(value)];
                    console.log('********************** updated source:      ' + JSON.stringify(source) + '\n\n)');
                    continue;
                }
            }
            source[key] = JSONExt.deepCopy(value);
        }
        return source;
    }
2023-02-06T17:43:59.168Z root INFO %%%% merge: source: {"configurations":[],"compounds":[],"version":"0.2.0"}
2023-02-06T17:43:59.168Z root INFO %%%% merge: target: {"version":"0.2.0","configurations":[{"name":"Launch Program","program":"${workspaceFolder}/app.js","request":"launch","skipFiles":["<node_internals>/**"],"type":"node"}]}
2023-02-06T17:43:59.168Z root INFO ********************** source[key] and value are both arrays
2023-02-06T17:43:59.168Z root INFO ********************** key          :       configurations
2023-02-06T17:43:59.168Z root INFO ********************** value target[key]    [{"name":"Launch Program","program":"${workspaceFolder}/app.js","request":"launch","skipFiles":["<node_internals>/**"],"type":"node"}]
2023-02-06T17:43:59.168Z root INFO ********************** source b4 update:    {"configurations":[],"compounds":[],"version":"0.2.0"}
2023-02-06T17:43:59.168Z root INFO ********************** updated source:      {"configurations":[{"name":"Launch Program","program":"${workspaceFolder}/app.js","request":"launch","skipFiles":["<node_internals>/**"],"type":"node"}],"compounds":[],"version":"0.2.0"}

)
2023-02-06T17:43:59.168Z root INFO 


2023-02-06T17:43:59.168Z root INFO %%%% merge: source: {"configurations":[{"name":"Launch Program","program":"${workspaceFolder}/app.js","request":"launch","skipFiles":["<node_internals>/**"],"type":"node"}],"compounds":[],"version":"0.2.0"}
2023-02-06T17:43:59.168Z root INFO %%%% merge: target: {"version":"0.2.0","configurations":[{"name":"Launch Program","program":"${workspaceFolder}/app.js","request":"launch","skipFiles":["<node_internals>/**"],"type":"node"}]}
2023-02-06T17:43:59.168Z root INFO ********************** source[key] and value are both arrays
2023-02-06T17:43:59.168Z root INFO ********************** key          :       configurations
2023-02-06T17:43:59.168Z root INFO ********************** value target[key]    [{"name":"Launch Program","program":"${workspaceFolder}/app.js","request":"launch","skipFiles":["<node_internals>/**"],"type":"node"}]
2023-02-06T17:43:59.168Z root INFO ********************** source b4 update:    {"configurations":[{"name":"Launch Program","program":"${workspaceFolder}/app.js","request":"launch","skipFiles":["<node_internals>/**"],"type":"node"}],"compounds":[],"version":"0.2.0"}
2023-02-06T17:43:59.168Z root INFO ********************** updated source:      {"configurations":[{"name":"Launch Program","program":"${workspaceFolder}/app.js","request":"launch","skipFiles":["<node_internals>/**"],"type":"node"},{"name":"Launch Program","program":"${workspaceFolder}/app.js","request":"launch","skipFiles":["<node_internals>/**"],"type":"node"}],"compounds":[],"version":"0.2.0"}

)
  • Operating System:
  • Theia Version:
@marcdumais-work marcdumais-work added the debug issues that related to debug functionality label Feb 6, 2023
@AlexandraBuzila
Copy link
Contributor

I can't assign the task to myself due to the repository permissions, but I would like to have a look at it.

@paul-marechal
Copy link
Member

Surprisingly debug configs are only duplicated whenever you have a folder opened as a workspace. If you have a .theia-workspace opened then it doesn't happen.

marcdumais-work added a commit that referenced this issue Feb 8, 2023
Since they fail all the time. This commit can be reverted once we have a
fix for #12153

Signed-off-by: Marc Dumais <[email protected]>
marcdumais-work added a commit that referenced this issue Feb 8, 2023
Since they fail all the time. This commit can be reverted once we have a
fix for #12153

Signed-off-by: Marc Dumais <[email protected]>
marcdumais-work added a commit that referenced this issue Feb 8, 2023
Since they fail all the time. This commit can be reverted once we have a
fix for #12153

Signed-off-by: Marc Dumais <[email protected]>
marcdumais-work added a commit that referenced this issue Feb 9, 2023
Since they fail all the time. This commit can be reverted once we have a
fix for #12153

Signed-off-by: Marc Dumais <[email protected]>
marcdumais-work added a commit that referenced this issue Feb 9, 2023
Since they fail all the time. This commit can be reverted once we have a
fix for #12153

Signed-off-by: Marc Dumais <[email protected]>
marcdumais-work added a commit that referenced this issue Feb 10, 2023
Since they consistently fail, following a recent change that uncovered a deeper bug
in the preferences.

This commit can be reverted once we have a fix for #12153

Signed-off-by: Marc Dumais <[email protected]>
marcdumais-work added a commit that referenced this issue Feb 17, 2023
Since they consistently fail, following a recent change that uncovered a deeper bug
in the preferences.

This commit can be reverted once we have a fix for #12153

Signed-off-by: Marc Dumais <[email protected]>
marcdumais-work added a commit that referenced this issue Feb 17, 2023
Since they consistently fail, following a recent change that uncovered a deeper bug
in the preferences.

This commit can be reverted once we have a fix for #12153

Signed-off-by: Marc Dumais <[email protected]>
marcdumais-work added a commit that referenced this issue Feb 17, 2023
Since they consistently fail, following a recent change that uncovered a deeper bug
in the preferences.

This commit can be reverted once we have a fix for #12153

Signed-off-by: Marc Dumais <[email protected]>
marcdumais-work added a commit that referenced this issue Feb 17, 2023
Since they consistently fail, following a recent change that uncovered a deeper bug
in the preferences.

This commit can be reverted once we have a fix for #12153

Signed-off-by: Marc Dumais <[email protected]>
marcdumais-work added a commit that referenced this issue Feb 20, 2023
Since they consistently fail, following a recent change that uncovered a deeper bug
in the preferences.

This commit can be reverted once we have a fix for #12153

Signed-off-by: Marc Dumais <[email protected]>
paul-marechal pushed a commit that referenced this issue Feb 22, 2023
* [browser tests] [launch preferences] Temporarily disable test suite

Since they consistently fail, following a recent change that uncovered a deeper bug
in the preferences.

This commit can be reverted once we have a fix for #12153

Signed-off-by: Marc Dumais <[email protected]>

* [browser tests] [saveable] - adapt to updated label

Opened editors referencing a deleted file have string (Deleted) after the file name.
It used to be (deleted). This commit adapts to the modified case.

Fixes #12081

Signed-off-by: Marc Dumais <[email protected]>

* [browser tests] [typescript] Solidify tests

Test cases 'references-view.find'/'references-view.findImplementations' is triggering a couple
of exceptions that result in error pop-ups in the app, that seem to intermittently derail the
following test cases. It happens when opening the references view directly, but not when
triggering the find references / find implementations, which in any case also
open the view.

In consequence, I have reorganized the order of operations to minimize issues: trigger
the commands first, then "open" the already opened view to get a handle on it and be able
to confirm that the content is what's expected.

Observing the failure points, added a bit more strategic "robustness" to the following TypeScript tests:
- editor.action.triggerSuggest
- Can execute code actions
- editor.action.quickFix

Also added a delay in beforeEach and afterEach handlers, to give time for code action contributions to be updated
when editors are closed and reopened. This helped with a few intermittent problems.

Signed-off-by: Marc Dumais <[email protected]>

* [browser tests] [find-replace] Solidify test

Add strategic delays to avoid a race condition that happens when files are
closed and opened in quick succession.

This test case opens and closes a '.js' file many times very quickly. The
specific file triggers the eslint and typescript-language-features plugins
to register code action handlers, that rely on the open file's to be referenced in a
structure maintained by the plugin system.

Code actions are immediately triggered upon the file being re-opened, and I think
previously registered handler do not always have enough time to be cleared, and end-up
throwing when the file can't be found in the plugin system's internal structure
(DocumentsExtImpl)

I tried but was not able to reproduce the issue manually, opening the file from
the explorer and closing it using shortcut shift-alt-w as quick as possible.

note: included a review suggestion by paul-marechal

Signed-off-by: Marc Dumais <[email protected]>

* [browser tests] [keybindings] Squash intermittent heisenbug

This one was tricky to find.

An exception would happen that failed the current test file, usually launch-preferences.spec.js,
only when running the full browser test suite. However, if that file was removed, the exception
would happen in another file, and another file...

Turns-out it's related to the vscode.json-language-features built-in extension and playing with
the package.json file.

It seems the problem is with the Language LinkProvider feature, and its usage by the
json-language-features extension. But I could not reproduce it outside of the test suite.

Here's what the exception looks-like:

root INFO   1) Launch Preferences
            "before all" hook in "Launch Preferences":
            Uncaught Error: Uncaught Error: There is no document for file:///home/<user>/theia/examples/browser/package.json

        Error: There is no document for file:///home/<user>/theia/examples/browser/package.json
            at LinkProviderAdapter.provideLinks (/home/<user>/theia/packages/plugin-ext/lib/plugin/languages/link-provider.js:31:35)
            at /home/<user>/theia/packages/plugin-ext/lib/plugin/languages.js:332:97
            at LanguagesExtImpl.withAdapter (/home/<user>/theia/packages/plugin-ext/lib/plugin/languages.js:123:20)
            at LanguagesExtImpl.$provideDocumentLinks (/home/<user>/theia/packages/plugin-ext/lib/plugin/languages.js:332:21)
            at /home/<user>/theia/packages/plugin-ext/lib/common/proxy-handler.js:91:71
            at processTicksAndRejections (node:internal/process/task_queues:96:5)
            at async RpcProtocol.handleRequest (/home/<user>/theia/packages/core/lib/common/message-rpc/rpc-protocol.js:167:28) (http://127.0.0.1:3000/vendors-node_modules_theia_monaco-editor-core_esm_vs_base_common_severity_js-node_modules_the-68fc42.js:1785)

I went with a simple fix: have the keybindings tests use an alternative file instead of package.json.

Signed-off-by: Marc Dumais <[email protected]>

* [browser tests] [browser-utils] [find-replace] increase test suites timeout

These test suites executes early, a the same time plugins are started. The default 2000ms
timeout would occasionally seemingly not be enough. Increasing it a bit.

e.g.:
2023-02-17T20:28:21.717Z root INFO   1 failing
2023-02-17T20:28:21.719Z root INFO   1) animationFrame
       should resolve after the given number of frames:
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

2023-02-20T14:54:45.051Z root INFO   1 failing
2023-02-20T14:54:45.051Z root INFO   1) Find and Replace
       "after each" hook for "Replace in the active explorer with the current editor":
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

Signed-off-by: Marc Dumais <[email protected]>

* [browser tests] Misc improvements in test infrastructure

Re-use puppeteer's empty Chrome tab:

Puppeteer at some point started to open the Chrome browser with an empty
tab by default. Then, when we create a new page for our tests, a new tab
would be added. At least When running the tests with UI
(option --test-inspect), sometimes the empty tab would be selected and
apparently cause many tests to fail. It's possible that this also sometimes
happened in headless mode.

To avoid this, now re-use the empty tab instead of creating a new one.

Make sure the Theia test app has focus and clear local browser storage:

When launching in non-headless mode (with a UI and dev-tools open), it looks-like
the dev-tools have focus, which interferes with some tests that query the UI,
expecting our app to be in focus. Simply clicking on the app before starting the
tests fixes it.

Also clear the local browser storage, to possibly avoid starting the app
with some state from the previous run. This could help when running the tests
locally, since CI should in theory always start with a clean environment.

Disable retry for failed tests in mocha config:

It only seems to make things worse.

Allow viewport to take available space instead of default 800x600:

This gives much more space for the Theia app, specially when running
in non-headless mode, where the editors can have very little real-estate
when views on the sides are open.

Delay exit after test finish:

When running the suite in headless mode, it exits as soon as the mocha tests
are done executing. In some cases, where there are lots of errors to report[1],
the final report did not have the chance to be printed-out, and so we were
left wondering about the details of what caused the failures.

[1]: basically the final pass/not pass/skip count as well as details about
the failed tests.

Note: includes a review suggestion by paul-marechal.

Signed-off-by: Marc Dumais <[email protected]>
@vince-fugnitto
Copy link
Member

Closed thanks to #12174.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug issues that related to debug functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants