Add new BeExactly assertions for DateTimeOffset #1609

gliljas · 2021-06-10T21:53:43Z

Not quite ready yet. More a basis for discussion.

The value formatting was one issue. I don't think the offset should be hidden, even if it's zero. It's no less significant than a positive or negative offset.

The assertion could perhaps be made with a Then step and a "...but the offsets differ" message.

Src/FluentAssertions/Formatting/DateTimeOffsetValueFormatter.cs

Src/FluentAssertions/Primitives/DateTimeOffsetAssertions.cs

dennisdoomen · 2021-06-11T07:56:30Z

Tests/FluentAssertions.Specs/Primitives/DateTimeOffsetAssertionSpecs.cs

+        #region (Not) BeExactly
+
+        [Fact]
+        public void Should_succeed_when_asserting_datetimeoffset_value_is_exactly_equal_to_the_same_value()


🔧 No need to mention datetimeoffset in there since it's clear from the context. Also, we prefer to keep literal names of code elements out of the test name.

Gladly. I just copied the Be/NotBe tests, which has the misleading "numeric" name, and changed it. Will fix.

dennisdoomen · 2021-06-11T07:56:57Z

Tests/FluentAssertions.Specs/Primitives/DateTimeOffsetAssertionSpecs.cs

+            Action act = () => dateTime.Should().BeExactly(sameDateTime);
+
+            // Assert
+            act.Should().NotThrow();


🔧 If the assertion is not expected to throw anything, you don't need to explicitly use NotThrow (anymore).

Src/FluentAssertions/Formatting/DateTimeOffsetValueFormatter.cs

jnyrup · 2021-06-13T19:06:07Z

Src/FluentAssertions/Primitives/DateTimeOffsetAssertions.cs

+                .FailWith("but found a <null> DateTimeOffset.")
+                .Then
                .ForCondition(Subject == expected)
+                .FailWith("but its value {0} doesn't.", Subject)


Suggested change

.FailWith("but its value {0} doesn't.", Subject)

.FailWith("but {0} does not.", Subject)

jnyrup · 2021-06-13T19:12:12Z

Src/FluentAssertions/Primitives/DateTimeOffsetAssertions.cs

        /// Zero or more objects to format using the placeholders in <paramref name="because" />.
        /// </param>
-        public AndConstraint<TAssertions> NotBe(DateTimeOffset unexpected, string because = "",
+        public AndConstraint<TAssertions> NotBeExactly(DateTimeOffset unexpected, string because = "",


Should the summary be updated?

jnyrup · 2021-06-13T19:13:55Z

Src/FluentAssertions/Primitives/DateTimeOffsetAssertions.cs

        {
            Execute.Assertion
-                .ForCondition(Subject == expected)
+                .ForCondition((Subject == null && expected == null) || (Subject != null && expected != null && Subject.Value.EqualsExact(expected.Value)))


We strive to keep lines at atmost 130 characters wide.

Suggested change

.ForCondition((Subject == null && expected == null) || (Subject != null && expected != null && Subject.Value.EqualsExact(expected.Value)))

.ForCondition((Subject == null && expected == null)

|| (Subject != null && expected != null && Subject.Value.EqualsExact(expected.Value)))

jnyrup

Aside from the failing tests, two other step are necessary to finish up this PR

Run AcceptChanges.ps1/sh to update the accepted public methods.
Add a line to the release notes.

Both steps are explained in https://github.com/fluentassertions/fluentassertions/blob/release-6.0/CONTRIBUTING.md, but ping me if anything is unclear or causing trouble.

jnyrup · 2021-06-22T12:23:16Z

Src/FluentAssertions/Primitives/DateTimeOffsetAssertions.cs

+                    .FailWith("but found a <null> DateTimeOffset.", expected)
+                    .Then
+                    .ForCondition(Subject == expected)
+                    .FailWith("but {1} does not.", expected, Subject)


Suggested change

.FailWith("but {1} does not.", expected, Subject)

.FailWith("but {0} does not.", Subject)

jnyrup · 2021-06-22T12:26:21Z

Src/FluentAssertions/Primitives/DateTimeOffsetAssertions.cs

+                .ForCondition(Subject.HasValue)
+                .FailWith("but found a <null> DateTimeOffset.")
+                .Then
+                .ForCondition(Subject.Value.EqualsExact(expected) == true)


I haven't checked the code out, but I would believe == true to be superfluous.

Suggested change

.ForCondition(Subject.Value.EqualsExact(expected) == true)

.ForCondition(Subject.Value.EqualsExact(expected))

jnyrup · 2021-06-22T12:27:30Z

Src/FluentAssertions/Primitives/DateTimeOffsetAssertions.cs

+                    .FailWith("but found a <null> DateTimeOffset.", expected)
+                    .Then
+                    .ForCondition(Subject.Value.EqualsExact(expected.Value))
+                    .FailWith("but it was {0}.", expected, Subject)


Suggested change

.FailWith("but it was {0}.", expected, Subject)

.FailWith("but it was {0}.", Subject)

jnyrup · 2021-06-22T12:27:38Z

Src/FluentAssertions/Primitives/DateTimeOffsetAssertions.cs

+                    .BecauseOf(because, becauseArgs)
+                    .WithExpectation("Expected {context:the date and time} to be exactly {0}{reason}, ", expected)
+                    .ForCondition(Subject.HasValue)
+                    .FailWith("but found a <null> DateTimeOffset.", expected)


Suggested change

.FailWith("but found a <null> DateTimeOffset.", expected)

.FailWith("but found a <null> DateTimeOffset.")

jnyrup · 2021-06-22T12:27:59Z

Src/FluentAssertions/Primitives/DateTimeOffsetAssertions.cs

+                    .BecauseOf(because, becauseArgs)
+                    .WithExpectation("Expected {context:the date and time} to represent the same point in time as {0}{reason}, ", expected)
+                    .ForCondition(Subject.HasValue)
+                    .FailWith("but found a <null> DateTimeOffset.", expected)


Suggested change

.FailWith("but found a <null> DateTimeOffset.", expected)

.FailWith("but found a <null> DateTimeOffset.")

jnyrup · 2021-06-22T12:34:17Z

Src/FluentAssertions/Primitives/DateTimeOffsetAssertions.cs

+            Execute.Assertion
+                .ForCondition(Subject?.EqualsExact(unexpected) != true)
+                .BecauseOf(because, becauseArgs)
                .FailWith("Did not expect {context:the date and time} to be {0}{reason}, but it was.", unexpected);


Suggested change

.FailWith("Did not expect {context:the date and time} to be {0}{reason}, but it was.", unexpected);

.FailWith("Did not expect {context:the date and time} to be exactly {0}{reason}, but it was.", unexpected);

jnyrup · 2021-06-22T12:34:45Z

Src/FluentAssertions/Primitives/DateTimeOffsetAssertions.cs

-                .ForCondition(Subject != unexpected)
+                .ForCondition(!((Subject == null && unexpected == null) || (Subject != null && unexpected != null && Subject.Value.EqualsExact(unexpected.Value))))
                .BecauseOf(because, becauseArgs)
                .FailWith("Did not expect {context:the date and time} to be {0}{reason}, but it was.", unexpected);


Suggested change

.FailWith("Did not expect {context:the date and time} to be {0}{reason}, but it was.", unexpected);

.FailWith("Did not expect {context:the date and time} to be exactly {0}{reason}, but it was.", unexpected);

dennisdoomen reviewed Jun 11, 2021

View reviewed changes

gliljas force-pushed the datetimeoffsetexact branch from 71ed70a to 2088ab0 Compare June 11, 2021 09:21

dennisdoomen requested a review from jnyrup June 11, 2021 11:52

jnyrup reviewed Jun 13, 2021

View reviewed changes

dennisdoomen approved these changes Jun 22, 2021

View reviewed changes

dennisdoomen requested a review from jnyrup June 22, 2021 12:15

jnyrup requested changes Jun 22, 2021

View reviewed changes

gliljas force-pushed the datetimeoffsetexact branch 2 times, most recently from c9eb1db to c27a434 Compare June 24, 2021 09:22

Added new BeExactly assertions for DateTimeOffset

c27a434

dennisdoomen requested a review from jnyrup June 29, 2021 06:42

jnyrup approved these changes Jun 29, 2021

View reviewed changes

dennisdoomen merged commit fdf0288 into fluentassertions:release-6.0 Jun 29, 2021

gliljas deleted the datetimeoffsetexact branch August 30, 2021 13:27

	.FailWith("but its value {0} doesn't.", Subject)
	.FailWith("but {0} does not.", Subject)

	.ForCondition((Subject == null && expected == null) \|\| (Subject != null && expected != null && Subject.Value.EqualsExact(expected.Value)))
	.ForCondition((Subject == null && expected == null)
	\|\| (Subject != null && expected != null && Subject.Value.EqualsExact(expected.Value)))

	.FailWith("but {1} does not.", expected, Subject)
	.FailWith("but {0} does not.", Subject)

	.ForCondition(Subject.Value.EqualsExact(expected) == true)
	.ForCondition(Subject.Value.EqualsExact(expected))

	.FailWith("but it was {0}.", expected, Subject)
	.FailWith("but it was {0}.", Subject)

	.FailWith("but found a <null> DateTimeOffset.", expected)
	.FailWith("but found a <null> DateTimeOffset.")

	.FailWith("Did not expect {context:the date and time} to be {0}{reason}, but it was.", unexpected);
	.FailWith("Did not expect {context:the date and time} to be exactly {0}{reason}, but it was.", unexpected);

Add new BeExactly assertions for DateTimeOffset #1609

Add new BeExactly assertions for DateTimeOffset #1609

Uh oh!

Conversation

gliljas commented Jun 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gliljas Jun 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnyrup left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gliljas commented Jun 10, 2021 •

edited

Loading

gliljas Jun 11, 2021 •

edited

Loading