Skip to content

Conversation

@Hoblovski
Copy link
Collaborator

@Hoblovski Hoblovski commented Aug 6, 2025

What type of PR is this?

This is a WIP attempting to make go test work.
Involving:

  • restructuring testdata/
  • refactoring *_test.go files. Including tests scaffolds, removing hardcoded paths etc.
  • whole-ast level checking (where to put the asts files? submodule or lfs with gz)

Check the PR title.

  • This PR title match the format: <type>(optional scope): <description>
  • The description of this PR title is user-oriented and clear enough for others to understand.
  • Attach the PR updating the user documentation if the current PR requires user awareness at the usage level. User docs repo

(Optional) Translate the PR title into Chinese.

修复文件,完事能用上 go test。这样无论我们 CI 还是别人 PR 都方便。

(Optional) More detailed description for this PR(en: English/zh: Chinese).

en:
zh(optional):

(Optional) Which issue(s) this PR fixes:

(optional) The PR that updates user documentation:

* lang/lsp/utils_test.go: no change
* lang/lsp/client_test.go: test basic LSP clients
* lang/lsp/lsp_test.go: removed because `TestDocumentSymbol_MarshalJSON` is covered in client_test.go
@Hoblovski Hoblovski changed the title (WIP) fix: pass all go tests (WIP) fix: pass all go tests in lang/ Aug 6, 2025
@Hoblovski
Copy link
Collaborator Author

Hoblovski commented Aug 6, 2025

status

To check, run

# TODO: go
bash -c '( set -ex ; for completed in ./lang/{lsp,collect,uniast,rust,} ; do echo $completed ; go test $completed ; done )'
  • lang/lsp/utils_test.go
  • lang/lsp/client_test.go
  • lang/lsp/lsp_test.go
  • lang/patch/lib_test.go
    • TestPatcher errors: absolute path in localsession.json.
  • lang/collect/collect_test.go
  • lang/uniast/ast_test.go
  • lang/rust/repo_test.go
  • lang/rust/ast_test.go
  • lang/rust/utils/lsp_test.go
  • lang/rust/rust_test.go
    • TestRustSpec_FunctionSymbol: deleted too intertwined, not suitable as unit test
  • errors lang/golang/parser/go_ast_test.go
    • TestCases errors: too large, contains obselete hardcoded json
  • lang/golang/writer/write_test.go need more tests: verify the written repo is correct
  • errors lang/golang/parser/pkg_test.go
    • Test_goParser_ParseRepo
    • Test_goParser_ParseDirs
    • TestGoAst
    • Test_matchMod
    • Test_goParser_ParseNode errors: trying to retrieve a node that is not present in the code?

@Hoblovski
Copy link
Collaborator Author

@AsterDY The PR is almost ready, would it be convenient for you to review next week?
Basically all the tests are passing.

Failing Tests

Package lang/patch

testdata/asts/localsession.json contains paths on your computer.
Replacing it with a freshly-generated ast would fix the issue.
(but we're not supposed to run the entire ast generation process in a unit test).

We could

  1. delete the test;
  2. put it in a system-level test;
  3. use a simpler testcase that does not involve dependencies.

Package lang/golang

  1. go_ast_test.go: hardcoded obselete json string
  2. pkg_test.go: tries to ParseNode(name=DeleteGls), but there is NO SUCH NODE

We could

  1. delete these tests and replace them with system-level ast checkers

Other caveats and potential inconsistencies

  1. The TestRustLSP/semanticTokens sometime fails, but testing it again would succeed.
  2. The StartOffset and EndOffset for Variables only include the variable name.
Expected (StartOffset:EndOffset):
      "IDC"
Got (Var.Content):
      "\nlazy_static! {\n    static ref IDC: &'static str = \"xx\";\n    static ref MY_ENV: String = std::env::var(\"MY_ENV\").unwrap();\n}\n"

@Hoblovski Hoblovski changed the title (WIP) fix: pass all go tests in lang/ fix: pass all go tests in lang/ Aug 8, 2025
@AsterDY
Copy link
Collaborator

AsterDY commented Aug 11, 2025

CI也顺便加一下?

@Hoblovski
Copy link
Collaborator Author

done

CI也顺便加一下?

@AsterDY AsterDY merged commit bb4eee7 into main Aug 12, 2025
3 checks passed
@Hoblovski Hoblovski deleted the fix/pass-all-go-tests branch August 15, 2025 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants