Exposing skip for evaluation by onsi

[gaffo]

Opening for dicussion for fixing #377

[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)?

[gaffo]

Which test file should I put em in?

…

On Oct 12, 2017 20:15, "Onsi Fakhouri" ***@***.***> 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 <#382 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAvLIkoZbDRtUOq8vhBiiRv9yntxn1kks5srtXqgaJpZM4P3wGu> .

[onsi]

i’d suggest doing it as an integration test - you should be able to follow the patterns in the /integration folder.

[gaffo]

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?

[onsi]

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.

[gaffo]

Okay thanks that's the advice I was looking for!

…

On Wed, Oct 25, 2017 at 9:47 PM, Onsi Fakhouri ***@***.***> 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 <#382 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAvLP28l1uYAuNSCVukyu6lyLmn5BoXks5swA7EgaJpZM4P3wGu> .

[williammartin]

@gaffo Looks like maybe this has been languishing, do you think you'll get a chance to implement Onsi's suggestions?

[gaffo]

Yes... Just waiting for time.

…

On Dec 5, 2017 12:35, "William Martin" ***@***.***> 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 <#382 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAvLE7RcIWrHotjyxUXe9QeMnog8iVwks5s9akfgaJpZM4P3wGu> .

[pmzajaczkowski]

It looks like all of the conditions are met to merge-in this pull request. Could you accept it? :)

[williammartin]

@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.