BoundfoxStudios / fluentassertions-unity

FluentAssertions for Unity
Apache License 2.0
54 stars 6 forks source link

Missing NumericAssertions for Unity and Unity.Mathematics types #9

Open Walter-Hulsebos opened 1 year ago

Walter-Hulsebos commented 1 year ago

FluentAssertions provides NumericAssertions that enable precise control over floating-point errors for numerical values. However, the current Unity fork of FluentAssertions does not implement these assertions for Unity's Vector types (i.e., Vector2, Vector3, Vector4) or Unity.Mathematics types (i.e., float2, float3, float4, double2, double3, double4, and matrices like float4x4).

Example:

float f = 69.000001f;
f.Should().BeApproximately(expectedValue: 69f, precision: 0.0001f); //Succeeds

This limitation severely impacts the usefulness of this package for testing mathematical operations in Unity, as it provides very little control over floating-point errors for these types. As a result, we are unable to leverage the full capabilities of this package for our Unity project.

I myself had these results come back from this test code:

// Arrange
F32x3 __vector   = new F32x3(x: 2, y: 4, z: 6);                         //Length = 7.483315
F32x3 __expected = new F32x3(x: 1.336306f, y: 2.672612f, z: 4.008918f); //Length = 5
const F32 MAX_LENGTH = 5;

// Act
F32x3 __clampedVector = __vector.WithMaxLength(maxLength: MAX_LENGTH);

// Assert
__clampedVector.Should().Be(__expected);

ReturnsClampedVectorWhenExceedMaxLength

Perhaps I'm missing something, and have simply overlooked some detail, but if not, could you please consider implementing NumericAssertions for numeric types from UnityEngine and Unity.Mathematics in this fork?

Thank you for your time and attention to this matter.

If you're busy just tell me an I could even make a PR myself.

ManuelRauber commented 1 year ago

Hi @Walter-Hulsebos

Thanks for bringing this up.

First, please note, that this repository is not a fork of FluentAssertions. It just wraps the original library and exposes the whole .NET Standard compatible API surface from FluentAssertions. Nothing more, nothing less.

Regarding your issue: It does work with Vector3 and float3 etc.. However the value is boxed first. That is not ideal and only solvable by extending this wrapper with specific assert functions for the types.

It still works because Should().Be() uses the underlying implementation of Object.Equals(), which is implemented in Vector3. But, Equals checks for exact representation of the Vector, which is not given in your case due to the nature of floating point precision.

If I use the same test as you (but using Vector3 instead of your types) and debug into them, I can see the accurate representation (because the one you see in the test runner is already rounded):

__clampedVector: (1.33630621, 2.67261243, 4.00891829)
__expected:      (1.33630598, 2.67261195, 4.00891781)

As you can see, they differ due to floating point precision.

Funny side note: If you use __expected == __clampedVector it will be true, because the == operator is overwritten by unity checking for approximate equality.

== and Equals work different for Vector3 than for float3. In float3 == and Equals is the same implementation and check for exact equality.

Ok, so. A real solution would be introducing more API, like FluentAssertion has for floats: Should().BeApproximately(). I need some more time to check how much work that would be, because there are a lot of types that need to be covered.

ManuelRauber commented 1 year ago

@Walter-Hulsebos I did some ground work and I think its doable in some hours. Take a look at this draft PR: https://github.com/BoundfoxStudios/fluentassertions-unity/pull/10

It mimics the implementation of FluentAssertions' NumericAssertions. Fortunately, all Unity types implement IEquatable and IFormatable - so we can use a common base for all of them.

So far, there should be: Should().

For nullables:

Implementation is done for Vector3 and float3 so far.

I put all Unity.Mathematics namespace types into another assembly, which also needs a scripting symbol to be set: FLUENTASSERTIONS_UNITY_MATHEMATICS. Otherwise I need to make the whole library depend on Unity.Mathematics, which I don't want.

Please let me know your thoughts.

Walter-Hulsebos commented 1 year ago

Looks great to me! Thanks a lot for the clarifications, I'd only been using the library for a short time, so I wasn't sure on if I was doing something wrong. It was something I encountered fairly quickly, after all.

Totally agree with the FLUENTASSERTIONS_UNITY_MATHEMATICS, things like this should never be a hard requirement. Thanks a bunch for your work!

ManuelRauber commented 1 year ago

Looks great to me! Thanks a lot for the clarifications, I'd only been using the library for a short time, so I wasn't sure on if I was doing something wrong. It was something I encountered fairly quickly, after all.

Totally agree with the FLUENTASSERTIONS_UNITY_MATHEMATICS, things like this should never be a hard requirement. Thanks a bunch for your work!

I started doing the following types:

All of them also got an extension method: BeCloseTo and NotBeCloseTo that simply does (value - expected).magnitude < maxDistance.

Please take a look if that somewhere close to your usecase because without generating all that stuff, it's a lot of work.

If you need more types, please consider doing a PR against the feature/unity-integration branch.

Not sure, if we can simply use a source generator to generate all the stuff. It will work until we need to do some extra stuff for specific types.

ManuelRauber commented 1 year ago

@Walter-Hulsebos Any comments?