Skip to content

Add a version of table.Entry that allows dumping the entry parameters.#689

Merged
onsi merged 1 commit into
onsi:masterfrom
fedepaol:configurableentryoutput
Jun 9, 2020
Merged

Add a version of table.Entry that allows dumping the entry parameters.#689
onsi merged 1 commit into
onsi:masterfrom
fedepaol:configurableentryoutput

Conversation

@fedepaol

@fedepaol fedepaol commented Jun 8, 2020

Copy link
Copy Markdown
Contributor

Sometimes, it's useful to have the parameters of a given table entry as part of the description of the It counterpart.
To achieve that, we add a new family of Entry constructor that accept a function instead of the description as a string.
That function needs to be implemented by the client of the library, and it's called with the list of arguments and returns a string that will be used as description of the given entry.

This was discussed in #688

I am not totally happy with the naming, but I guess keeping it short-ish has a value here.

@fedepaol fedepaol force-pushed the configurableentryoutput branch from eefb527 to a0905ed Compare June 8, 2020 22:12
@onsi

onsi commented Jun 8, 2020

Copy link
Copy Markdown
Owner

Thanks @fedepaol - this is in the right direction but now I'm realizing why we might actually want to use some magic. The current implementation is not symmetric with the existing implementation of DescribeTable. Specifically, the function passed to DescribeTable has a signature that describes the inputs of the test (e.g. from the test you've added: func(x int, y int, expected bool) however the function passed to DEntry will be forced to have a less descriptive signature: func(params []interface{}) string that will force the user to refer to the parameters by by index instead of by name.

So I think we actually want DEntry to take interface{} (just like DescribeTable does) and allow the user to pass a function that more closely parallels their DescribeTable function.

At that point one could argue that we don't need DEntry and can just go with having Entry do the right thing depending on the type of input it has... but I don't feel too strongly about that.

WDYT?

@fedepaol fedepaol force-pushed the configurableentryoutput branch from a0905ed to bce59f1 Compare June 8, 2020 23:33
@fedepaol

fedepaol commented Jun 8, 2020

Copy link
Copy Markdown
Contributor Author

The new version is a bit more complex on code, but I do agree, it looks a bit cleaner on client site.
Please take a look.

@onsi

onsi commented Jun 9, 2020

Copy link
Copy Markdown
Owner

This looks great, @fedepaol - I think all it needs now is to be documented (inline in the comments, but also on the gh-pages branch here: https://github.com/onsi/ginkgo/blob/gh-pages/index.md#table-driven-tests )

If you're up for adding some documentation let me know. Otherwise, I'll pull this in and document it myself.

@fedepaol

fedepaol commented Jun 9, 2020

Copy link
Copy Markdown
Contributor Author

This looks great, @fedepaol - I think all it needs now is to be documented (inline in the comments, but also on the gh-pages branch here: https://github.com/onsi/ginkgo/blob/gh-pages/index.md#table-driven-tests )

If you're up for adding some documentation let me know. Otherwise, I'll pull this in and document it myself.

Sure, I'll do! TBH, it was pretty late yesterday and I wasn't able to find the docs :P

Sometimes, it's useful to have the parameters of a given table entry as part of the description of the It counterpart.
To achieve that, we allow passing a function as the description parameter.
In case a string is passed, the previous behaviour is preserved. In case of function, it is expected to return a string
that will be used as the new description for the generated It.
@fedepaol

fedepaol commented Jun 9, 2020

Copy link
Copy Markdown
Contributor Author

@onsi done! Feel free to correct it.

@onsi

onsi commented Jun 9, 2020

Copy link
Copy Markdown
Owner

Looks great! I’ll merge these in when i get to my desk.

@onsi onsi merged commit 21eaef2 into onsi:master Jun 9, 2020
@fedepaol

fedepaol commented Jun 9, 2020

Copy link
Copy Markdown
Contributor Author

Thanks! When do you expect this to be available as a tag?

@onsi

onsi commented Jun 9, 2020

Copy link
Copy Markdown
Owner

I expect in the next couple of days or so

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