ginkgo
ginkgo copied to clipboard
onsi
Exposing skip for evaluation by onsi
@gaffo this seems like a reasonable enough approach to me. Yeah, it's clunky, but it's also explicit and relatively simple so I'm in favor. Wanna add some tests and some documentation (under the gh-pages branch)?
Which test file should I put em in?
On Oct 12, 2017 20:15, "Onsi Fakhouri" [email protected] wrote:
@gaffo https://github.com/gaffo this seems like a reasonable enough approach to me. Yeah, it's clunky, but it's also explicit and relatively simple so I'm in favor. Wanna add some tests and some documentation (under the gh-pages branch)?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/onsi/ginkgo/pull/382#issuecomment-336338981, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAvLIkoZbDRtUOq8vhBiiRv9yntxn1kks5srtXqgaJpZM4P3wGu .
i’d suggest doing it as an integration test - you should be able to follow the patterns in the /integration folder.
Any specific file I should dump it in in integration, next to something else? do I need to check the error text is correct or just make sure it compiles and runs correctly?
Actually - now that I think of it DescribeSkip might be confusing since there is a Skip already. Could we mirror Gomega and use DescribeWithOffset etc.?
I think you could then create a new integration/offset_test.go. And yeah I think we'd want to make sure the error text is correct to catch regressions. An easy way to do that is make a new fixture, run the test, grab the line number, then just hard code that in the test. You should be able to pattern match and copy/paste your way to victory with the integration tests but let me know if I can help.
Okay thanks that's the advice I was looking for!
On Wed, Oct 25, 2017 at 9:47 PM, Onsi Fakhouri [email protected] wrote:
Actually - now that I think of it DescribeSkip might be confusing since there is a Skip already. Could we mirror Gomega https://github.com/onsi/gomega/blob/master/gomega_dsl.go#L137 and use DescribeWithOffset etc.?
I think you could then create a new integration/offset_test.go. And yeah I think we'd want to make sure the error text is correct to catch regressions. An easy way to do that is make a new fixture, run the test, grab the line number, then just hard code that in the test. You should be able to pattern match and copy/paste your way to victory with the integration tests but let me know if I can help.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/onsi/ginkgo/pull/382#issuecomment-339550057, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAvLP28l1uYAuNSCVukyu6lyLmn5BoXks5swA7EgaJpZM4P3wGu .
@gaffo Looks like maybe this has been languishing, do you think you'll get a chance to implement Onsi's suggestions?
Yes... Just waiting for time.
On Dec 5, 2017 12:35, "William Martin" [email protected] wrote:
@gaffo https://github.com/gaffo Looks like maybe this has been languishing, do you think you'll get a chance to implement Onsi's suggestions?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/onsi/ginkgo/pull/382#issuecomment-349432971, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAvLE7RcIWrHotjyxUXe9QeMnog8iVwks5s9akfgaJpZM4P3wGu .
It looks like all of the conditions are met to merge-in this pull request. Could you accept it? :)
@pmzajaczkowski Changes have been requested earlier in the comment chain. If you'd like to tackle those and open a separate PR I'd be happy to accept that.