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

analysis::type_check_tests::test and rename::tests::test fail as of 0.12.2 #868

Closed
GaetanLepage opened this issue Nov 20, 2024 · 11 comments · Fixed by #903
Closed

analysis::type_check_tests::test and rename::tests::test fail as of 0.12.2 #868

GaetanLepage opened this issue Nov 20, 2024 · 11 comments · Fixed by #903

Comments

@GaetanLepage
Copy link

Describe the bug

Context: bumping tinymist frorm 0.12.0 to 0.12.2 in nixpkgs.

Two tests are failing on the 0.12.2 release.
Are you able to replicate those failures or is it only on our side ?

failures:

---- analysis::type_check_tests::test stdout ----
To update snapshots run `cargo insta review`
thread 'analysis::type_check_tests::test' panicked at /private/tmp/nix-build-tinymist-0.12.2.drv-0/tinymist-0.12.2-vendor/insta-1.41.1/src/glob.rs:128:9:
glob! resulted in 1 snapshot assertion failure

---- rename::tests::test stdout ----
To update snapshots run `cargo insta review`
thread 'rename::tests::test' panicked at /private/tmp/nix-build-tinymist-0.12.2.drv-0/tinymist-0.12.2-vendor/insta-1.41.1/src/glob.rs:128:9:
glob! resulted in 1 snapshot assertion failure


failures:
    analysis::type_check_tests::test
    rename::tests::test

test result: FAILED. 45 passed; 2 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.23s
@GaetanLepage GaetanLepage changed the title analysis::type_check_tests::test and rename::tests::test fail as of 0.12.2 analysis::type_check_tests::test and rename::tests::test fail as of 0.12.2 Nov 20, 2024
@GaetanLepage
Copy link
Author

Complete log: https://paste.glepage.com/raw/spider-wasp-pug

@Myriad-Dreamin
Copy link
Owner

It must be passed at least when it was released. The CI was running tests only on Linux x64. Do we have log?

@GaetanLepage
Copy link
Author

I was able to reproduce those failures on both x86_64-linux and aarch64-darwin.
We are building tinymist at the 0.12.2 git tag.

@Myriad-Dreamin
Copy link
Owner

From the log, all tests are actually passed. The errors are probably caused by recent changes of insta crate. I also met these assertion failure recently in local.

@Myriad-Dreamin
Copy link
Owner

I fixed some of them in tinymist v0.12.2+ but I'm not sure whether I fixed format of all snapshots. https://github.com/Myriad-Dreamin/tinymist/pull/831/files

@Myriad-Dreamin
Copy link
Owner

@GaetanLepage could you skip these two tests? We could check them again in tinymist v0.12.4, which will be published in this weekend.

@GaetanLepage
Copy link
Author

I tried building at this commit and had:

failures:

---- completion::tests::test_pkgs stdout ----
To update snapshots run `cargo insta review`
thread 'completion::tests::test_pkgs' panicked at /build/tinymist-0.12.2-vendor/insta-1.41.1/src/glob.rs:128:9:
glob! resulted in 1 snapshot assertion failure
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- inlay_hint::tests::smart stdout ----
To update snapshots run `cargo insta review`
thread 'inlay_hint::tests::smart' panicked at /build/tinymist-0.12.2-vendor/insta-1.41.1/src/glob.rs:128:9:
glob! resulted in 1 snapshot assertion failure


failures:
    completion::tests::test_pkgs
    inlay_hint::tests::smart

test result: FAILED. 45 passed; 2 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.18s

I will skip the two tests mentioned in this issue and we can re-try at the next release. Thank you for your quick help !

@GaetanLepage
Copy link
Author

GaetanLepage commented Nov 20, 2024

Ok it is actually worse than I thought. The tests are failing non-deterministically.

Here are the results of running the exact same build several times in a row on different systems:

  • x86_64-linux
    • Run 1
      • rename::tests::test
    • Run 2
      • analysis::post_type_check_tests::test
      • semantic_tokens_full::tests::test
    • Run 3
      • inlay_hint::tests::smart
    • Run 4
      • folding_range::tests::test
      • references::tests::test
  • aarch64-linux
    • Run 1
      • analysis::call_info_tests::test
      • semantic_tokens_full::tests::test
    • Run 2
      • folding_range::tests::test
      • inlay_hint::tests::smart
    • Run 3
      • analysis::module_tests::test
      • folding_range::tests::test
  • x86_64-darwin
    • Run 1
      • inlay_hint::tests::smart
      • rename::tests::test
    • Run 2
      • analysis::post_type_check_tests::test
      • inlay_hint::tests::smart
    • Run 3
      • analysis::post_type_check_tests::test
      • analysis::type_check_tests::test
  • aarch64-darwin
    • Run 1
      • analysis::type_check_tests::test
      • inlay_hint::tests::smart
    • Run 2
      • hover::tests::test
      • rename::tests::test
    • Run 3
      • references::tests::test
      • rename::tests::test

@GaetanLepage
Copy link
Author

Unfortunately, those tests are still failing in our build process with 0.12.4.
Do you think that this can be on your side or on ours (nixpkgs) ?
I wouldn't like to keep an issue open here if it happens to be an us-problem.

@Myriad-Dreamin
Copy link
Owner

Hi, I'm sorry that I cannot help a lot on issues about nix. I have run it again for 10 times on my x86_64-windows and x86_64-linux PC and no errors is thrown. I then conclude that some setting specific to nix should be wrong when running insta snapshot tests. Regard to the log you posted above, the assertion failures are just false positive:

Complete log: https://paste.glepage.com/raw/spider-wasp-pug

If you have further questions, please feel free to ask me again.

@GaetanLepage
Copy link
Author

Ok no worry, I think that we will just disable those tests.
Thank you for your help :)

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

Successfully merging a pull request may close this issue.

2 participants