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

test(lsp): inconsistent go_to_definition_for_paths test #4211

Closed
eightfilms opened this issue Mar 2, 2023 · 3 comments · Fixed by #6315
Closed

test(lsp): inconsistent go_to_definition_for_paths test #4211

eightfilms opened this issue Mar 2, 2023 · 3 comments · Fixed by #6315
Assignees
Labels
bug Something isn't working language server LSP server testing General testing

Comments

@eightfilms
Copy link
Contributor

https://github.com/FuelLabs/sway/actions/runs/4310274360/jobs/7518550643

This test failed on the first run, but passed on triggering a re-run.

@eightfilms eightfilms added bug Something isn't working language server LSP server testing General testing labels Mar 2, 2023
@esdrubal
Copy link
Contributor

esdrubal commented Mar 2, 2023

These two unrelated PRs also had the same issue:
https://github.com/FuelLabs/sway/actions/runs/4296935289/jobs/7489282684
https://github.com/FuelLabs/sway/actions/runs/4315841590/jobs/7530795852

@sdankel sdankel self-assigned this Mar 6, 2023
@sdankel
Copy link
Member

sdankel commented Mar 6, 2023

Happened again with additional logging: https://github.com/FuelLabs/sway/actions/runs/4347930396/jobs/7595847115

---- go_to_definition_for_paths stdout ----
thread 'go_to_definition_for_paths' panicked at 'Failed to deserialize response: Null input: Request {
    jsonrpc: Version,
    method: "textDocument/definition",
    params: Some(
        Object {
            "position": Object {
                "character": Number(39),
                "line": Number(6),
            },
            "textDocument": Object {
                "uri": String("file:///home/runner/work/sway/sway/sway-lsp/tests/fixtures/tokens/paths/src/main.sw"),
            },
        },
    ),
    id: Some(
        Number(
            61,
        ),
    ),
} error: data did not match any variant of untagged enum GotoDefinitionResponse', sway-lsp/tests/integration/lsp.rs:342:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Looks like it's not finding goto definitions for dfun on this line: use deep_mod::deeper_mod::deep_fun as dfun;

@sdankel sdankel assigned JoshuaBatty and unassigned sdankel Mar 7, 2023
sdankel added a commit that referenced this issue Mar 7, 2023
## Description

This test was flaky in CI:
https://github.com/FuelLabs/sway/actions/runs/4310274360/jobs/7518550643

The error message wasn't super helpful, so this PR adds more information
to the error. If it flakes again, we'll have more to go on. This will at
least make it possible to tell which of the test cases failed.

Related #4211

## Checklist

- [x] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
sdankel added a commit that referenced this issue Mar 7, 2023
## Description

Disabling this test since it's failing on a lot of PRs. Will re-enable
it once it's fixed.

Related #4211

## Checklist

- [x] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
@sdankel
Copy link
Member

sdankel commented Jun 26, 2023

We could try switching to cargo-nextest to get retries on flaky tests like this one. It should also speed up CI times in the sway repo overall.

esdrubal pushed a commit that referenced this issue Aug 13, 2024
…d out tests (#6315)

## Description
closes #4211
closes #6258

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working language server LSP server testing General testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants