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

Request for describe comments to be matched with -goblin.run regexp #105

Open
alexey-komarov opened this issue Jun 29, 2021 · 4 comments
Open

Comments

@alexey-komarov
Copy link

I'd like to run particular group of tests described via Describe(), just for developing purposes when I want to debug entire group, so i would really appreciate a separate flag, e.g -goblin.runFullName with the following behavior:

diff --git a/goblin.go b/goblin.go
index a4617db..078965c 100644
--- a/goblin.go
+++ b/goblin.go
@@ -284,8 +284,30 @@ func (g *G) SetReporter(r Reporter) {
        g.reporter = r
 }
 
+func (g *G) GetFullName(name string) string {
+       parent := g.parent
+       names := []string{name}
+
+       for parent != nil {
+               names = append(names, parent.name)
+               parent = parent.parent
+       }
+
+       result := ""
+       for _, n := range names {
+               if len(result) == 0 {
+                       result = n
+                       continue
+               }
+
+               result = n + "." + result
+       }
+
+       return result
+}
+
 func (g *G) It(name string, h ...interface{}) {
-       if matchesRegex(name) {
+       if matchesRegex(g.GetFullName(name)) {
                it := &It{name: name, parent: g.parent, reporter: g.reporter}
                if g.parent == nil {
                        panic(fmt.Sprintf("It(\"%s\") block should be written inside Describe() block.", name))
@marcosnils
Copy link
Member

I like this idea. Do we need a separate flag though? Why not just using the current run flag and just change the current matchesRegex line to use the GetFullName function that you're proposing?

I don't think we're breaking backwards compatibility here, but just allowing to target Describe blocks as it's name will be part of the regex target now.

@alexey-komarov
Copy link
Author

@marcosnils
Using one flag would be better for sure, but i always think about those who already using this feature in their pipelines, don't want to break anything :)

@marcosnils
Copy link
Member

I don't think this is a very big deal since the run flag is mostly used for development purposes and not generally CI pipelines. In any case, we can always release a new major.

Would you like to open a PR so we can get this merged?

One thing that I believe it's missing here is to keep recursing all the way up until parent == nil. Since you can have multiple nested describe blocks: i.e:

Describe("Some", func() {
  Describe("Little", func() {
     Describe("Rabbit", func() {
         It("jumps", func(){})
     })
  })
}) 

So the ultimate run regex should target "Some.Little.Rabbit.jumps" and if I read the code correctly it's only targetting "Rabbit.jumps" given the current patch.

Additionally, does it make sense to concatenate the blocks with .? Why not : or any other char? I don't have any specific preferences here, just wondering if it's the best we can do about it.

@shakefu
Copy link
Contributor

shakefu commented Sep 2, 2021

I'd like to see this but concatenated with spaces to match Mocha's fullTitle() behavior

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

3 participants