cedarbdd / cedar

BDD-style testing using Objective-C
http://groups.google.com/group/cedar-discuss
1.19k stars 140 forks source link

Raise errors when dsl is misused #395

Closed tjarratt closed 8 years ago

tjarratt commented 8 years ago

Fixes #394.

The first commit is the brunt of this feature work. DSL misuse encompasses two main failure modes:

In writing this feature I tried to avoid adding much more scope to Cedar's public interface, but found it necessary to expose CDRdisableSpecValidation() and CDRenableSpecValidation() functions so that we could keep our existing tests that create cedar examples by calling it() inside of a test. I also thought it desirable to allow someone to disable this feature, if they were using Cedar in some interesting way.

Then, once I was totally done with this feature, writing a happy commit message and running all of the test suite ... I discovered a completely unrelated regression in the Type Utilities part of Cedar. The commit message for that is pretty verbose -- I recommend reading it if you want to know when a valid pointer sometimes can be misconstrued to be a NULL pointer. It was quite a find.

modocache commented 8 years ago

Cool! Quick added something similar in https://github.com/Quick/Quick/pull/448, and I think that reduced the number of issues being reported on the project.

tjarratt commented 8 years ago

@idoru I'd be curious to know if you have any feedback or thoughts around this PR. I've attempted to be a bit more conservative than I normally would when refactoring and testing with mocks, but felt like some of this was necessary.

I've been feeling some pain around the maintainability of CDRfunctions.{h|m} and feel like starting to extract some of this to Obj-C when possible would make testing parts of the implementation a tad easier, although I suspect you could teach me a thing or two about testing Cedar with Cedar 😉

idoru commented 8 years ago

@tjarratt thanks for doing this work! I had a few naming nit-picks which I've commented on c1f33d2.

Regarding 2ddf819... How interesting. It's interesting that we chose to compare with '\0' to check for a NULL pointer. Perhaps we should compare with (void *)0?

Checkout §6.3.2.3 point 3 in particular: http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1124.pdf

tjarratt commented 8 years ago

Thanks for all the feedback, @idoru. Brian commented on an outdated diff that comparison to NULL would be better than '\0'. The definition for NULL is a bit hard to follow, but it seems like it will defined as ((void*)0) when C++ is not defined, and 0 otherwise.

NULL seems like a perfectly cromulent constant to here use, but I'm open to switching it to an explicit (void *)0 too.