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

Output spec type, name, and codelocation - for use by an IDE #753

Closed
dlipovetsky opened this issue Dec 21, 2020 · 11 comments
Closed

Output spec type, name, and codelocation - for use by an IDE #753

dlipovetsky opened this issue Dec 21, 2020 · 11 comments

Comments

@dlipovetsky
Copy link
Contributor

dlipovetsky commented Dec 21, 2020

Once there are more than a few specs in a file, navigating to a particular spec, or getting an overview of all the specs, is not easy manually, and is not possible (as far as I know) with an IDE, e.g., VSCode.

Take, for example, https://github.com/onsi/ginkgo/blob/efb9e6987c00c280f4c3c624a105a9f6c27f2122/internal/spec/spec_test.go. If each spec were a standard go test, it would included in the list of top-level symbols, because the test is a function, e.g. TestThisFeature. In Ginkgo, a spec is a function call, e.g., It(...).

It's worth noting that both standard go tests and ginkgo specs can be defined at runtime, e.g., as is the case with table-driven tests. I think it would still be helpful to quickly navigate to the line where that happens.

Here's a comparison of navigating to a symbol in VSCode, using a test from the standard librarytime package, and a test from the internal/spec package:

image

image

I'd like to be able to output a list or tree structure with all of the containers and specs, their types, text, and code locations, for a specific file. That would be enough information to provide to an IDE like VSCode, where an extension could make use of it.

@dlipovetsky
Copy link
Contributor Author

dlipovetsky commented Dec 21, 2020

I've tried two experiments to generate the tree of all containers and specs, their types, text, and code locations, for a specific file.

  1. Parse the file and walk the AST. Upsides: No dependency on ginkgo. Downsides: Might not work with your ginkgo version, if the syntax changes (unlikely). It's not straightforward to get the text of a spec; although it's almost always a string literal, but sometimes it is not.
  2. Re-use the tree built by internal/suite. Upsides: Could be a new ginkgo command ,e.g., ginkgo outline. Guaranteed to work with your ginkgo version. Easy to get the tree! Downsides: test code must be compiled and evaluated to generate the tree. The internal tree structure doesn't make the spec types available.

@onsi
Copy link
Owner

onsi commented Dec 22, 2020

@dlipovetsky I love this idea and I'm super supportive of making it a reality - though I don't have a ton of time on my hands to make it so these days.

I agree that a new ginkgo sub-command would make sense here. And I think it's reasonable for a plugin for Ginkgo tests to take a hard dependency on the ginkgo cli and either the tree-parsing approach or the AST approach could be folded into that subcommand. I'd be inclined to start with the AST approach as that would have a chance of working when the suite is malformed and cannot formally compile (e.g. you're working on a test in one file and leave it in a half-broken state but then want to navigate to ta different test in a different file within the same package). As you say, the text of a spec is not always a string literal but I think that edge case is OK and would be intuitive to most users/developers.

So... a sub-command that takes a package or lists of packages as an argument and spits out a JSON description of the tree would be a great addition. Are you interested in working on a PR? I'd be happy to merge it in and try to help - and we could drive it out with a few integration tests to ensure the functionality doesn't break going forward.

@dlipovetsky
Copy link
Contributor Author

Thanks for the feedback, @onsi!

I'd be inclined to start with the AST approach as that would have a chance of working when the suite is malformed and cannot formally compile ...

+1

As you say, the text of a spec is not always a string literal but I think that edge case is OK and would be intuitive to most users/developers.

+1

Are you interested in working on a PR?

Yes. I've got a first iteration AST approach working. I'll clean it up a little, wrap a sub-command around it, PR it, and continue to iterate on that PR.

Sample input:

package p

import (
        "fmt"

        . "github.com/onsi/ginkgo"
)

var _ = Describe("group 1", func() {
        Context("context 1", func() {
                It("should test 1.1", func() {
                })
                It("should test 1.2", func() {
                })
        })
        Context("context 2", func() {
                It("should test 2.1", func() {
                })
                It(fmt.Sprintf("should test %d.%d", 2, 2), func() {
                })
        })
})

Sample output. (I'm constructing a tree, because that can be easily mapped to a "tree view," and that kind of view makes it easy to see the grouping of specs. This is just the tree marshalled to JSON, with indentation. Other outputs are possible.)

{
  "SpecType": "root",
  "CodeLocation": "",
  "Text": "",
  "Children": [
    {
      "SpecType": "Describe",
      "CodeLocation": "src.go:9:9",
      "Text": "group 1",
      "Children": [
        {
          "SpecType": "Context",
          "CodeLocation": "src.go:10:2",
          "Text": "context 1",
          "Children": [
            {
              "SpecType": "It",
              "CodeLocation": "src.go:11:3",
              "Text": "should test 1.1",
              "Children": null
            },
            {
              "SpecType": "It",
              "CodeLocation": "src.go:13:3",
              "Text": "should test 1.2",
              "Children": null
            }
          ]
        },
        {
          "SpecType": "Context",
          "CodeLocation": "src.go:16:2",
          "Text": "context 2",
          "Children": [
            {
              "SpecType": "It",
              "CodeLocation": "src.go:17:3",
              "Text": "should test 2.1",
              "Children": null
            },
            {
              "SpecType": "It",
              "CodeLocation": "src.go:19:3",
              "Text": "[could not determine]",
              "Children": null
            }
          ]
        }
      ]
    }
  ]
}

@onsi
Copy link
Owner

onsi commented Dec 23, 2020

That looks great @dlipovetsky - thanks so much for working on this :)

A couple of quick thoughts:

  • I think each node in the tree could also describe whether it is focused or pending. It is often useful to "find all pending specs" or "take me to the focused test."
  • There are a few aliases in Ginkgo (e.g. Specify for It and When for Context and Describe) - those'll need to get captured as well.
  • I think rather than "[could not determine]" simply setting Text to null would convey the same notion, no?
  • If possible, I think it would also be useful to capture any By statements as children of It nodes.

I'm working on a Ginkgo 2.0 and I don't think anything in the works will break this output format. One thing we'll be adding is support for tags - and I imagine it'll be nice for tags to also appear in the tree view json structure... but, again, that's a future thing :)

dlipovetsky added a commit to dlipovetsky/ginkgo that referenced this issue Dec 26, 2020
dlipovetsky added a commit to dlipovetsky/ginkgo that referenced this issue Dec 26, 2020
Implements feature request in onsi#753

Signed-off-by: Daniel Lipovetsky <daniel.lipovetsky@gmail.com>
dlipovetsky added a commit to dlipovetsky/ginkgo that referenced this issue Dec 26, 2020
dlipovetsky added a commit to dlipovetsky/ginkgo that referenced this issue Dec 26, 2020
dlipovetsky added a commit to dlipovetsky/ginkgo that referenced this issue Dec 26, 2020
@dlipovetsky
Copy link
Contributor Author

Thanks for this feedback. All good ideas!

  • I think each node in the tree could also describe whether it is focused or pending. It is often useful to "find all pending specs" or "take me to the focused test."

Done, including backpropagation of unfocus, and propagation of inherited focus/pending property 😅

  • There are a few aliases in Ginkgo (e.g. Specify for It and When for Context and Describe) - those'll need to get captured as well.

Done.

  • I think rather than "[could not determine]" simply setting Text to null would convey the same notion, no?

I decided to use undefined for now, but maybe null connotes the right thing, i.e., "don't know."

  • If possible, I think it would also be useful to capture any By statements as children of It nodes.

Done.

onsi pushed a commit that referenced this issue Dec 30, 2020
…file (#754)

* Adds 'outline' command to print the outline of specs/containers in a file

Implements feature request in #753

* outline: Add support for nodot and alias import of the ginkgo package

* outline: During a post-order traversal of an AST Node, do not derive its ginkgo metadata

The post-order traversal needs to check whether the AST Node is a ginkgo node.
That can be done by comparing the positions of the AST Node and the last
visited ginkgo node. Deriving the ginkgo metadata is not necessary.

* outline: Add backpropagation of unfocus, propagation of inherited focus/pending properties

* outline: Add instructions and script for creating/updating sample test results

To make it easier to maintain the outline tests.

* outline: Use script to re-create result samples for all tests

The content has not changed; only the JSON formatting and the filenames have
changed.

* outline: Factor the back/propagation code to helper functions

The top-level function needed to be shorter.

* outline: Refactor deriving ginkgo identifier from call expression

Again, the top-level function needed to be shorter.

* outline: Move the exported outline code into its own file

Keep the unexprted ginkgoNode code in a separate file.

* outline: Instead of aliasing ginkgoNode with outline, embed ginkgoNode in outline

An advantage of embedding is that it does not require casting. The outline
receivers does not change.

* outline: (fix) Derive the "text" of the By ginkgo call

The "text" was being ignored by mistake.

* outline: Test an outline of a "suite_test.go" file

The test and its result samples were already present, but the test was not run.

* outline: Add "indent" format

This is really just for fun. Here's an example:

``shell
ginkgo outline -format=indent outline/_testdata/normal_test.go
```

```shell
Name,Text,Start,End,Spec,Focused,Pending
Describe,NormalFixture,116,605,false,false,false
    Describe,normal,152,244,false,false,false
        It,normal,182,240,true,false,false
            By,step 1,207,219,false,false,false
            By,step 2,223,235,false,false,false
    Context,normal,247,307,false,false,false
        It,normal,276,303,true,false,false
    When,normal,310,367,false,false,false
        It,normal,336,363,true,false,false
    It,normal,370,396,true,false,false
    Specify,normal,399,430,true,false,false
    Measure,normal,433,480,true,false,false
```

It also happens to look nice piped through `column`:

```shell
ginkgo outline -format=indent outline/_testdata/normal_test.go | column --table -s=","
```

```
Name            Text           Start  End  Spec   Focused  Pending
Describe        NormalFixture  116    605  false  false    false
    Describe    normal         152    244  false  false    false
        It      normal         182    240  true   false    false
            By  step 1         207    219  false  false    false
            By  step 2         223    235  false  false    false
    Context     normal         247    307  false  false    false
        It      normal         276    303  true   false    false
    When        normal         310    367  false  false    false
        It      normal         336    363  true   false    false
    It          normal         370    396  true   false    false
    Specify     normal         399    430  true   false    false
    Measure     normal         433    480  true   false    false
```
@dlipovetsky
Copy link
Contributor Author

dlipovetsky commented Dec 31, 2020

Documentation PR: #757

@dlipovetsky
Copy link
Contributor Author

Add support for the "table" extension: #758

@dlipovetsky
Copy link
Contributor Author

I'm going to call this done! Thanks for the feedback and reviews, @onsi!

@onsi
Copy link
Owner

onsi commented Jan 20, 2021

hey @dlipovetsky this outline stuff is great - and i'm excited to play with your vscode plugin when you're ready to share ;)

i'm wondering if you'd be interested in joining as a Ginkgo & Gomega committer? Can always use help engaging with issues and PRs and investing in Ginkgo and Gomega - any thoughts?

@dlipovetsky
Copy link
Contributor Author

@onsi My apologies--I missed your last message! I just came back to this issue to share an update.

Development took a bit, but I made steady progress over the past month, and I just published the extension! It's got an outline view and a go-to-symbol menu. I'd love to get your feedback. Two ways to install:

  1. Marketplace: https://marketplace.visualstudio.com/items?itemName=dlipovetsky.ginkgo-tools
  2. Extension .vsix file: https://github.com/dlipovetsky/vscode-ginkgo-tools/releases/ (in the release artifacts)

Honestly, I'm flattered that you'd ask if I'd be interested to become a committer! What kind of expectations do you have for committters? I just want to make sure I could meet them.

@onsi
Copy link
Owner

onsi commented Feb 10, 2021

hey @dlipovetsky - super cool! I'll take a look 😄

there aren't any formal expectations or anything - I'd say:

  • help triage and respond to issues and PRs as they come in
  • help maintain code, especially code you may have contributed to
  • periodically collaborate on cutting a new release

in addition there's a ginkgo channel on the gophers slack instance as well as a ginkgo-internal channel that some of us use to chat about ginkgo. both are low traffic and you'd be welcome to join either but wouldn't have to.

right now i'm heads down making headway on v2 - that's going to pull in a lot of changes in the coming weeks/months as i start to put it in a public branch and, eventually, merge things into master so there may be a fair bit of activity in the near-term but i expect things will level off after that.

thoughts?

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

No branches or pull requests

2 participants