Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TEST_ASSERT_EQUAL_FLOAT will always pass if the expected value is infinity #4

Closed
jvranish opened this issue Jul 26, 2012 · 8 comments
Closed

Comments

@jvranish
Copy link
Member

float const expected = 1.0f/0.0f;
TEST_ASSERT_EQUAL_FLOAT(expected, 12345.0f); // <-- this will always pass
TEST_ASSERT(expected == 12345.0f); // <-- this will fail

The cause is in unity_internals.h:

define UNITY_TEST_ASSERT_EQUAL_FLOAT(expected, actual, line, message) UNITY_TEST_ASSERT_FLOAT_WITHIN((_UF)(expected) * (_UF)UNITY_FLOAT_PRECISION, (_UF)expected, (_UF)actual, (UNITY_LINE_TYPE)line, message)

Specifically: (_UF)(expected) * (_UF)UNITY_FLOAT_PRECISION
as 1.#INF00 * UNITY_FLOAT_PRECISION still equals 1.#INF00

So we end up checking that our value equals infinity plus or minus infinity.

@rryles
Copy link

rryles commented Oct 30, 2012

I'd be happy to look into a fix for this. However we'd need some agreement on what the behaviour should be. I'd suggest the following:

TEST_ASSERT_EQUAL_FLOAT(123.4f, Inf) Fails
TEST_ASSERT_EQUAL_FLOAT(Inf, 123.4f) Fails
TEST_ASSERT_EQUAL_FLOAT(Inf,Inf) Passes
TEST_ASSERT_EQUAL_FLOAT(Inf,-Inf) Fails
TEST_ASSERT_EQUAL_FLOAT(Inf,NaN) fails
TEST_ASSERT_EQUAL_FLOAT(NaN,Inf) fails

I'm not sure what we should do here though:

TEST_ASSERT_EQUAL_FLOAT(NaN1,NaN1)?
TEST_ASSERT_EQUAL_FLOAT(NaN1,NaN2)?

@mvandervoord
Copy link
Member

I think I'd personally prefer to have all Inf / -Inf / NaN comparisons fail in TEST_ASSERT_EQUAL_FLOAT, and have a TEST_ASSERT_IS_INF, TEST_ASSERT_IS_NEG_INF, and TEST_ASSERT_IS_NAN. It seems cleaner, especially since there's rarely a supported standard notation for any of those values.

@sw17ch
Copy link
Contributor

sw17ch commented Oct 30, 2012

I tend to agree with Mark here.

On Tue, Oct 30, 2012 at 10:39 AM, Mark VanderVoord <notifications@github.com

wrote:

I think I'd personally prefer to have all Inf / -Inf / NaN comparisons
fail in TEST_ASSERT_EQUAL_FLOAT, and have a TEST_ASSERT_IS_INF,
TEST_ASSERT_IS_NEG_INF, and TEST_ASSERT_IS_NAN. It seems cleaner,
especially since there's rarely a supported standard notation for any of
those values.


Reply to this email directly or view it on GitHubhttps://github.com//issues/4#issuecomment-9907916.

@rryles
Copy link

rryles commented Oct 30, 2012

The only think I might disagree with is TEST_ASSERT_EQUAL(Inf,Inf). MY reasoning is that TEST_ASSERT_TRUE(Inf==Inf) would pass. Also, if someone has passed infinity as an expected value they probably want it to pass if the actual value is infinity. However I'm perfectly happy to implement either way.

As an aside I'm guessing we don't want to introduce a dependancy on math.h? That shouldn't be a problem though.

@rryles
Copy link

rryles commented Oct 31, 2012

What about the array versions? Should they also fail for any special values? We probably want consistent behaviour with the non-array versions.

Is there a need for the likes of TEST_ASSERT_FLOAT_ARRAY_IS_INF?

@mvandervoord
Copy link
Member

The array versions should fail for the same things that the non-array versions do, for sure.

I'm feeling like an array check for IS_INF or IS_NAN is overkill.

@rryles
Copy link

rryles commented Oct 31, 2012

If someone really wants to check for an array of NaNs they can always roll their own loop.

I've changed the array versions so they have the same behaviour.

@unity3diy
Copy link

do you have its webplaer to test ?

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

No branches or pull requests

5 participants