Refactoring tests for better readability

[gergelyorosz]

I use tests as to understand what a project does - this is a refactor to make that understanding easier.

Using the given / when / then naming convention, as well as (when possible) one assert per test.

After doing this refactor, reading over the tests gives me a good idea of what the extensions do. (and on that note I'm wondering if "maybe" expresses what it does... but yet to find a better name):

[RuiAAPeres]

On the onNone, you replaced it with:

test_onSome_whenInvokedOnNonNilValue_thenOperationIsNotInvoked_andValueIsReturned
test_onSome_whenInvokedOnNilValue_thenOperationIsInvoked_andNilIsReturned

I think it should be (following your line of thought):

test_onNone_whenInvokedOnNonNilValue_thenOperationIsNotInvoked_andValueIsReturned
test_onNone_whenInvokedOnNilValue_thenOperationIsInvoked_andNilIsReturned

[gergelyorosz]

@RuiAAPeres hawk eyes! spot on with that comment, thanks for highlighting it. I changed that.

[RuiAAPeres]

Looking at:

test_maybe_whenInvokedOnNonNilValue_withNonNilPredicate_thenExecutesOperationAndReturnsItsResult
test_maybe_whenInvokedOnNilValue_withNilPredicate_thenDoesNotExecuteOperationAndReturnsPredicate

Specially:

_withNonNilPredicate_
_withNilPredicate_

It's slight misleading, because the predicate (T -> U) is not an optional ((T -> U)?). It would be enough to have it like:

test_maybe_whenInvokedOnNonNilValue_thenExecutesOperationAndReturnsItsResult
test_maybe_whenInvokedOnNilValue_thenDoesNotExecuteOperationAndReturnsDefaultValue

You also:

_thenDoesNotExecuteOperationAndReturnsPredicate

It should be:

_thenDoesNotExecuteOperationAndReturnsDefaultValue

[gergelyorosz]

@RuiAAPeres you're right on both ends. This was me making sense of "maybe", and not removing that part.

Just let me know please if you see some more inconsitencies - read through them again and hopefully not missed much else.

[RuiAAPeres]

In some cases you use:

 let number: Int? = 3

But in others:

 let nonNilledNumber: Int? = 3

[gergelyorosz]

@RuiAAPeres great on noticing that as well 👍 I've updated this to be consistent with the naming in the tests - so for the first two tests left it as number as it's not imporant there if this is a nil number or not. Updated everywhere else.

In those cases changing number to nonNilledNumber would be confusing IMO. Also let me know if you think reverting nonNilledNumber to number would make more sense (I'm unsure if either is better - one is more explicit about testing with an optional that's not nil, the other is implicit)

[RuiAAPeres]

🍰