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

Discrepancies in PR 3286 #3304

Closed
akavel opened this issue Feb 25, 2022 · 1 comment · Fixed by #3308
Closed

Discrepancies in PR 3286 #3304

akavel opened this issue Feb 25, 2022 · 1 comment · Fixed by #3308
Assignees

Comments

@akavel
Copy link
Contributor

akavel commented Feb 25, 2022

After the first round of QA/Acceptance Review testing on PR #3286 (https://www.pivotaltracker.com/story/show/180916527), focusing on the library/getPackage message, I found some discrepancies between what is described in the design doc, what happens actually, and what is described in the API docs. They fall mostly in two general categories:

  1. The yaml example from Design Doc does not work verbatim; various tweaks seem needed, and I'm not sure why and whether they are documented anywhere;
  2. The format of the response from library/getPackage seems to not match the format described in the API doc, both in field names and in field types (at various levels of nesting).

A list of example cases I observed comes below.

API documentation of ComponentGroups.newGroups field name is inconsistent with .new field being apparently returned instead

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":{"new":[{"module":"Gruuup 1","exports":["foo"]}]},"raw":{"name":"Footest","component-groups":{"new":[{"Gruuup 1":null,"exports":["foo"]}]}}}}

Yaml semantics different than in Design Doc

Snippet similar as presented in Design Doc does not work:

name: Footest
component-groups:
  new:
    - Gruuup 1:
        exports:
          - foo

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":null,"raw":{"name":"Footest","component-groups":{"new":[{"Gruuup 1":{"exports":["foo"]}}]}}}}

Instead, snippet with different indentation (which conveys different semantics in YAML, as can be seen comparing the "raw" field in the response below vs. above) works:

name: Footest
component-groups:
  new:
    - Gruuup 1:
      exports:
        - foo

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":{"new":[{"module":"Gruuup 1","exports":["foo"]}]},"raw":{"name":"Footest","component-groups":{"new":[{"Gruuup 1":null,"exports":["foo"]}]}}}}

Similarly, adding a shortcut as in design doc does not work:

name: Footest
component-groups:
  new:
    - Gruuup 1:
      exports:
        - foo:
            shortcut: f

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":null,"raw":{"name":"Footest","component-groups":{"new":[{"Gruuup 1":null,"exports":[{"foo":{"shortcut":"f"}}]}]}}}}

but instead semantically different YAML document seems to work:

name: Footest
component-groups:
  new:
    - Gruuup 1:
      exports:
        - foo:
          shortcut: f

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":{"new":[{"module":"Gruuup 1","exports":[{"name":"foo","shortcut":"f"}]}]},"raw":{"name":"Footest","component-groups":{"new":[{"Gruuup 1":null,"exports":[{"foo":null,"shortcut":"f"}]}]}}}}

Nonconforming response returned

In response, at path result.componentGroups.new[0].exports[0] note a string value "foo", where expected a Component containing {"name":"foo"}.

name: Footest
component-groups:
  new:
    - Gruuup 1:
      exports:
        - foo

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":{"new":[{"module":"Gruuup 1","exports":["foo"]}]},"raw":{"name":"Footest","component-groups":{"new":[{"Gruuup 1":null,"exports":["foo"]}]}}}}

In response, at path result.componentGroups.new[0].exports[1] note a string value "bar", where expected a Component containing {"name":"bar"}.

name: Footest
component-groups:
  new:
    - Gruuup 1:
      exports:
        - foo:
          shortcut: f
        - bar

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":{"new":[{"module":"Gruuup 1","exports":[{"name":"foo","shortcut":"f"},"bar"]}]},"raw":{"name":"Footest","component-groups":{"new":[{"Gruuup 1":null,"exports":[{"foo":null,"shortcut":"f"},"bar"]}]}}}}

Empty response returned with no error message, even though component groups at least partially correct

Is it described somewhere how we want to behave in case of partially correct input? Do we want to somehow provide error message to user to guide them what they did wrong? Notably, the extends: section in the document below was copied from the design doc.

name: Footest
component-groups:
  new:
    - Gruuup 1:
      color: '#aabbcc'
      icon: some-icon
      exports:
        - foo:
          shortcut: f
        - bar
    - Le gruppe 2:
      exports:
        - foo
  extends:
    - Base.Group 2:
      exports:
        - foo

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":null,"raw":{"name":"Footest","component-groups":{"new":[{"Gruuup 1":null,"color":"#aabbcc","icon":"some-icon","exports":[{"foo":null,"shortcut":"f"},"bar"]},{"Le gruppe 2":null,"exports":["foo"]}],"extends":[{"Base.Group 2":null,"exports":["foo"]}]}}}}

API docs for extendedGroups field don't match actual semantics

see https://github.com/enso-org/enso/blob/abbb3a467915eabf37e91329317ba6c3274fc1e0/docs/language-server/protocol-language-server.md#componentgroups
In example below:

  1. Instead of .extendedGroup, field named extends is returned
  2. At result.componentGroups.extends[0].module, expected a ModuleReference object according to API docs, but got a string "Standard.Base.Main" instead
name: Footest
component-groups:
  extends:
    - Standard.Base.Main:
      exports:
      - name: foo

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":{"extends":[{"module":"Standard.Base.Main","exports":["foo"]}]},"raw":{"name":"Footest","component-groups":{"extends":[{"Standard.Base.Main":null,"exports":[{"name":"foo"}]}]}}}}

Error code should be 8007, is 1000, for LocalLibraryNotFound

Per definition of LocalLibraryNotFound message, referenced in the errors section of library/getPackage message. Got code 1000 in transcript below:

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Fooblah","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"error":{"code":1000,"message":"Local library [local.Fooblah] has not been found."}}

Returned componentGroups field's empty value is sometimes {}, sometimes null

Not strictly error, but possibly surprising behavior.

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":{},"raw":{"name":"Footest"}}}

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":{},"raw":{"name":"Footest","component-groups":null}}}

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":{},"raw":{"name":"Footest","component-groups":{"new":null}}}}

{"jsonrpc":"2.0","id":5,"method":"library/getPackage","params":{"namespace":"local","name":"Footest","version":{"type":"LocalLibraryVersion"}}}
{"jsonrpc":"2.0","id":5,"result":{"license":null,"componentGroups":null,"raw":{"name":"Footest","component-groups":{"new":["Gruuup 1"]}}}}
@akavel
Copy link
Contributor Author

akavel commented Feb 28, 2022

Today, I tried to test the other command introduced by PR #3286, editions/listDefinedComponents. I didn't seem to be successful with this. However, it's not currently clear to me if I did my testing incorrectly, or if the functionality is not working as I expected. For the record, my attempts went as described below:

  1. Modified distribution/lib/Standard/Base/0.0.0-dev/package.yaml, adding the following section at the bottom of the file:

    component-groups:
      new:
        - Testme 1:
          exports:
            - Main.Nothing
    
  2. Run rm -rf built-distribution/

  3. Run sbt, then execute the following two commands in it:

    • runtime/clean
    • buildEngineDistribution

    then exit sbt.

  4. Run:

    $ ./built-distribution/enso-engine-0.0.0-dev-macos-amd64/enso-0.0.0-dev/bin/enso --server --root-id 6f7d58dd-8ee8-44cf-9ab7-9f0454033641 --rpc-port 30616 --data-port 30617 --path $HOME/enso/projects/Unnamed_2
    
  5. In a separate window, run: $ websocat ws://127.0.0.1:30616, then pass the following messages to it, each time waiting until response shows up:

    • {"jsonrpc":"2.0","id":0,"method":"session/initProtocolConnection","params":{"clientId":"7ed3e4d3-db69-451a-b919-4cb5bc95b3f1"}}
    • {"jsonrpc":"2.0","id":4,"method":"editions/listDefinedComponents","params":{"edition":{"type":"NamedEdition","editionName":"0.0.0-dev"}}}

The following response was observed from the last message:

{"jsonrpc":"2.0","id":4,"result":{"availableComponents":[]}}

At the same time, it can be seen, that component-groups section is present in the corresponding package.yaml in built-distribution/:

$ cat built-distribution/enso-engine-0.0.0-dev-macos-amd64/enso-0.0.0-dev/lib/Standard/Base/0.0.0-dev/package.yaml
version: 0.0.0-dev
namespace: Standard
name: Base
license: APLv2
authors:
- name: Enso Team
  email: [email protected]
component-groups:
  new:
  - Testme 1: null
    exports:
    - Main.Nothing
maintainers:
- name: Enso Team
  email: [email protected]

I'm happy to retry the last test if advised that I did something incorrectly or missed some step when performing it.

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