drunkcod / Cone

8 stars 1 forks source link

Can't verify Expression of type AndAlso #14

Closed lucaminudel closed 13 years ago

lucaminudel commented 13 years ago

this code raise that error

        bool devsCustomConfig = false;
        bool othersCustomConfig = false;

        // ...

        Verify.That(() => devsCustomConfig == true && othersCustomConfig == false);

with this change it work

Verify.That(() => (devsCustomConfig & othersCustomConfig == false) == true);


drunkcod commented 13 years ago

I'm a bit ambivalent with regards to what the "correct" behavior should really be here. Clearly such a verification asserts multiple things at once, and should from a "puritan" point of view be written as two separate verifications. That also gives better localization when a regression happens. The alternative would be to do this split implicitly and simply indicate what part of the compound expression failed.

Your thoughts on this?

lucaminudel commented 13 years ago

agree is likely that this error is caused by an assertion that is testing 2 things at once. in that case the error message can suggest to split the assertions into 2 distinct Verify.That.

maybe something like syntax exception with a message like "verify each side of the And expression with a distinct Verify.That" ?

kitofr commented 13 years ago

+1 In my personal opinion I would prefer such statements to be written as separate asserts/verifications. In reference i would not write:

config.has_custom_configuration?.should be_true && config.has_some_other_configuration?.should be_true

in ruby... because that would be to long read better on 2 separate lines :)

neochrome commented 13 years ago

I agree that it's probably "more right" to have separate verifications. I also very much appreciate one of the core benefits of using Cone: one gets to write verifications using plain code. And to me it's seems a little strange that "ordinary" logical expressions breaks. Therefore I think I'll vote for full support of verifying using such expressions. :)

drunkcod commented 13 years ago

After pondering this for a while I've concluded that from a 'Cone perspective' neochrome is right. The aim is to be as close to 'real' code as possible so this should be allowed.

The advice still remains to treat it as a smell. Support will be part of the next release.