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

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

Merged
merged 13 commits into from
Dec 30, 2020

Conversation

dlipovetsky
Copy link
Contributor

@dlipovetsky dlipovetsky commented Dec 26, 2020

Why this PR is needed:
Implements feature request in #753.

Please see the test fixtures for example CSV and JSON output.

Special notes:
There are a couple of TODOs in the code, and I'll call them out here:

  • Aliased and nodot imports are not yet supported.
  • The Focus property is derived only from the container or spec name. It is not inherited from an outer container. Also, the focusing behavior described in the docs (and implemented here) is not yet supported:

Nested programmatically focused specs follow a simple rule: if
a leaf-node is marked focused, any of its ancestor nodes that
are marked focus will be unfocused.

@dlipovetsky
Copy link
Contributor Author

First CI run failed all jobs, because I hadn't run go mod tidy before committing. I ran locally and amended.

Second CI run failed just the "tip" job, because that version of go mod wanted to remove some more entries from go.sum. I removed those by hand locally and amended.

@dlipovetsky
Copy link
Contributor Author

Third CI run failed just the "tip" job. Looks unrelated to my changes:

• Failure [30.014 seconds]

Running Specs when passed a number of packages to run [It] should run the ginkgo style tests 

/home/travis/gopath/src/github.com/onsi/ginkgo/integration/run_test.go:72

  Timed out after 30.001s.

  Expected process to exit.  It did not.

  /home/travis/gopath/src/github.com/onsi/ginkgo/integration/run_test.go:74

@onsi
Copy link
Owner

onsi commented Dec 26, 2020

Thanks @dlipovetsky - this is super exciting! I'll take a look in the next few days. I suspect the integration test is flaky (possibly because of a change in go tip, but more likely it's just flakey and the travis worker it was on was slow).

…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.
…t results

To make it easier to maintain the outline tests.
The content has not changed; only the JSON formatting and the filenames have
changed.
The top-level function needed to be shorter.
Again, the top-level function needed to be shorter.
Keep the unexprted ginkgoNode code in a separate file.
…e in outline

An advantage of embedding is that it does not require casting. The outline
receivers does not change.
The "text" was being ignored by mistake.
The test and its result samples were already present, but the test was not run.
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
```
@@ -0,0 +1,61 @@
// Copyright 2013 The Go Authors. All rights reserved.
Copy link
Contributor Author

@dlipovetsky dlipovetsky Dec 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you see any problem with copy/pasting upstream code here. I've kept the upstream license, and noted the source commit.

// > leaf-node is marked focused, any of its ancestor nodes that are marked
// > focus will be unfocused.
func (n *ginkgoNode) BackpropagateUnfocus() {
focusedSpecInSubtreeStack := []bool{}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is hard to understand. I need to add some explanatory comments, and probably a unit test.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the whole back propagate thing is intrinsically complex! thanks for making this work!!

if !ok {
// Because `Nodes` calls this function only when the node is an
// ast.CallExpr, this should never happen
panic(fmt.Errorf("node starting at %d, ending at %d is not an *ast.CallExpr", node.Pos(), node.End()))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if panicking is the only option. This is supposed to be impossible to reach.

currentIndent := 0
pre := func(n *ginkgoNode) {
b.WriteString(fmt.Sprintf("%*s", currentIndent, ""))
b.WriteString(fmt.Sprintf("%s,%s,%d,%d,%t,%t,%t\n", n.Name, n.Text, n.Start, n.End, n.Spec, n.Focused, n.Pending))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered moving this to a ginkgoNode.String() receiver, but because most of the work is "opinionated" formatting, I thought it best not to put it there.

@onsi
Copy link
Owner

onsi commented Dec 30, 2020

@dlipovetsky this is amazing, thanks so much! I'm going to merge this in. Would you be up for updating the documentation on the gh-pages branch and I can pull that in as well? Just a simple description of the command would be enough imo.

I may do a bit of clean-up and move the tests over to the integration test suite. But I didn't want to delay so am pulling this in now! Thanks so much!!

@onsi onsi merged commit 071c369 into onsi:master Dec 30, 2020
@dlipovetsky
Copy link
Contributor Author

Would you be up for updating the documentation on the gh-pages branch and I can pull that in as well? Just a simple description of the command would be enough imo.

Will do!

I may do a bit of clean-up and move the tests over to the integration test suite.

That sounds right. Thanks!

dlipovetsky added a commit to dlipovetsky/ginkgo that referenced this pull request Dec 31, 2020
The `ginkgo outline` command was added in
onsi#754.
onsi pushed a commit that referenced this pull request Dec 31, 2020
The `ginkgo outline` command was added in
#754.
This was referenced Mar 17, 2021
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 this pull request may close these issues.

2 participants