kcl-lang / kcl

KCL Programming Language (CNCF Sandbox Project). https://kcl-lang.io
https://kcl-lang.io
Apache License 2.0
1.61k stars 113 forks source link

[Refractor] Refractor lsp unit test #1379

Closed Wck-iipi closed 4 months ago

Wck-iipi commented 4 months ago

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

2. What is the scope of this PR (e.g. component or file name):

/kclvm/tools/src/LSP/

3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):

4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):

5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:

Wck-iipi commented 4 months ago

@Peefy @He1pa

  1. I have just refractored one function, are these the required changes you are looking for? If yes, I would do this to all of them.
  2. Regarding goto_def_test_snapshot macro, wouldn't it be better to check the state like start, end and file path (like here) rather than the entire object? I am saying this as this is giving the following which will be impossible to test against. image

    Or do you have something else in mind regarding this snapshot.

He1pa commented 4 months ago

@Peefy @He1pa

  1. I have just refractored one function, are these the required changes you are looking for? If yes, I would do this to all of them.
  2. Regarding goto_def_test_snapshot macro, wouldn't it be better to check the state like start, end and file path (like here) rather than the entire object? I am saying this as this is giving the following which will be impossible to test against.

image Or do you have something else in mind regarding this snapshot.

Yes, that's exactly what I want. For the content of snapshot, I just provide an example. You can design the format by yourself. As long as it contains the information needed for the test (scalar/array/link, file, pos, etc.).

He1pa commented 4 months ago

@Peefy @He1pa

  1. I have just refractored one function, are these the required changes you are looking for? If yes, I would do this to all of them.
  2. Regarding goto_def_test_snapshot macro, wouldn't it be better to check the state like start, end and file path (like here) rather than the entire object? I am saying this as this is giving the following which will be impossible to test against.

image Or do you have something else in mind regarding this snapshot.

You need to pay attention to the absolute path of the snapshot path, which will cause CI to fail. I provide a simple version in https://github.com/kcl-lang/kcl/pull/1381, you can optimize it