Atrium: improve specs for iterable, use one time iterables

Created on 18 Sep 2019  Â·  24Comments  Â·  Source: robstoll/atrium

Platform (jvm, jdk8, js, android): all

Code related feature

Currently the specs use listOf which can be consumed many times.
Might well be there is a hidden bug. Let us use one time consumable iterables instead to find out.

enhancement help wanted
Source
picture robstoll

Most helpful comment

i hope i can be of help here.

picture lufafa123 on 19 Sep 2019
👍2

All 24 comments

i hope i can be of help here.

lufafa123 picture lufafa123 on 19 Sep 2019
👍2

@nlufafa as this is your first contribution to Atrium, it is maybe a good idea to have a look at Your first code contribution.

  • The specs are under /misc/specs/atrium-specs-common/src/main/kotlin/ch/tutteli/atrium/specs/integration/ e.g. IterableContainsInAnyOrderAtLeast1EntriesAssertionsSpec.kt
  • you basically need to exchange oneToFour, oneToSeven and oneToSevenNullable in IterableContainsSpecBase
  • moreover you need to adopt the usages in the specs. For instance, in IterableContainsInAnyOrderAtLeast1EntriesAssertionsSpec there is
    val fluent = expect(oneToSeven)
    you need to inline this (CTRL+ALT+N in intellij)

Let me know if you need additional hints or help and do not hesitate to create a WIP PR

robstoll picture robstoll on 19 Sep 2019

@nlufafa as you have not yet forked the repo I was wondering if you need help. Let me know if this is the case and have a look at Your first code contribution

Edit unassigning the task as you did not react and others might want to take it up

robstoll picture robstoll on 29 Sep 2019

I'll work on this. I was going to pick up #169 but someone beat me to it

aljacinto picture aljacinto on 9 Oct 2019

@robstoll to turn it into one time consumable, should I just turn it to this:

val oneToFour = { listOf(1.0, 2.0, 3.0, 4.0, 4.0).asIterable() }

then turn the expect to expect(oneToFour()) ?

aljacinto picture aljacinto on 11 Oct 2019

The goal is that the Iterable can only be consumed once. List can be consumed as many times as you want. You can use the following: sequenceOf(1.0, 2.0, 3.0, 4.0,4.0).constrainOnce().asIterable()

robstoll picture robstoll on 11 Oct 2019

@robstoll I made the changed above but now getting "the sequence can be consumed only once." as expected because some of the suites have multiple tests that use the same val. Can I initialize it beforeEachTest? or is that why you suggested inlining it? Does that mean inlining the creation of sequence itself? i.e.

expect(sequenceOf(1.0, 2.0, 3.0, 4.0,4.0).constrainOnce().asIterable()).allFun ...

the above means deleting the oneToFour, etc. from IterableContainsSpecBase

aljacinto picture aljacinto on 14 Oct 2019

I actually think we could combine our ideas to get the best results. Change oneToFour to 

val oneToFour = { sequenceOf(1.0, 2.0, 3.0, 4.0,4.0).constrainOnce().asIterable() }

And in the suites you inline fluent (I think most of the time it is called like that) so that in the it("..."){..} part it looks like the following:

it("....") {
   expect(oneToFour()) ...
}
robstoll picture robstoll on 14 Oct 2019

@robstoll I made the changes but it's failing on the two specs below. What are these specs for?

    include(object : AssertionCreatorSpec<Iterable<Double>>(
        describePrefix, oneToSeven(),
        all.forAssertionCreatorSpec("$isGreaterThanDescr: 0.0") { isGreaterThan(0.0) }
    ) {})
    include(object : AssertionCreatorSpec<Iterable<Double?>>(
        "$describePrefix[nullable Element] ", oneToSeven(),
        allNullable.forAssertionCreatorSpec("$isGreaterThanDescr: 0.0") { isGreaterThan(0.0) }
    ) {})

error message:

expect the thrown exception: java.lang.IllegalStateException
◆ is instance of type: AssertionError (java.lang.AssertionError)
  » ▶ message: CANNOT evaluate representation as it is based on subject which is not defined.
        » is instance of type: String (kotlin.String) -- Class: String (java.lang.String)
        » contains: 
          ⚬ value: "at least one assertion defined: false"        <174021077>
            ⚬ ▶ number of occurrences: -1
                ◾ is at least: 1
          ⚬ value: "You forgot to define assertions in the assertionCreator-lambda"        <1257652548>
            ⚬ ▶ number of occurrences: -1
                ◾ is at least: 1
          ⚬ value: "Sometimes you can use an alternative to `{ }` For instance, instead of `toThrow<..> { }` you should use `toThrow<..>()`"        <1682167706>
            ⚬ ▶ number of occurrences: -1
                ◾ is at least: 1
        » does not contain: 
          ⚬ value: "is greater than: 0.0"        <77113070>
            ⚬ ▶ number of occurrences: -1
                ◾ is: 0        (kotlin.Int <1538290135>)
  » Properties of the unexpected IllegalStateException
    » message: "This sequence can be consumed only once."        <590669700>
    » stacktrace: 
      ⚬ kotlin.sequences.ConstrainedOnceSequence.iterator(SequencesJVM.kt:23)
      ⚬ kotlin.sequences.SequencesKt___SequencesKt$asIterable$$inlined$Iterable$1.iterator(Iterables.kt:94)
      ⚬ kotlin.collections.CollectionsKt___CollectionsKt.toCollection(_Collections.kt:1155)
      ⚬ kotlin.collections.CollectionsKt___CollectionsKt.toMutableList(_Collections.kt:1188)
      ⚬ kotlin.collections.CollectionsKt___CollectionsKt.toList(_Collections.kt:1179)
      ⚬ ch.tutteli.atrium.domain.robstoll.lib.creating.IterableAssertionsKt$_iterableAll$1.invoke(iterableAssertions.kt:40)
      ⚬ ch.tutteli.atrium.domain.robstoll.lib.creating.IterableAssertionsKt$_iterableAll$1.invoke(iterableAssertions.kt)
      ⚬ ...
ch.tutteli.atrium.reporting.AtriumError: expect the thrown exception: java.lang.IllegalStateException
◆ is instance of type: AssertionError (java.lang.AssertionError)
  » ▶ message: CANNOT evaluate representation as it is based on subject which is not defined.
        » is instance of type: String (kotlin.String) -- Class: String (java.lang.String)
        » contains: 
          ⚬ value: "at least one assertion defined: false"        <174021077>
            ⚬ ▶ number of occurrences: -1
                ◾ is at least: 1
          ⚬ value: "You forgot to define assertions in the assertionCreator-lambda"        <1257652548>
            ⚬ ▶ number of occurrences: -1
                ◾ is at least: 1
          ⚬ value: "Sometimes you can use an alternative to `{ }` For instance, instead of `toThrow<..> { }` you should use `toThrow<..>()`"        <1682167706>
            ⚬ ▶ number of occurrences: -1
                ◾ is at least: 1
        » does not contain: 
          ⚬ value: "is greater than: 0.0"        <77113070>
            ⚬ ▶ number of occurrences: -1
                ◾ is: 0        (kotlin.Int <1538290135>)
  » Properties of the unexpected IllegalStateException
    » message: "This sequence can be consumed only once."        <590669700>
    » stacktrace: 
      ⚬ kotlin.sequences.ConstrainedOnceSequence.iterator(SequencesJVM.kt:23)
      ⚬ kotlin.sequences.SequencesKt___SequencesKt$asIterable$$inlined$Iterable$1.iterator(Iterables.kt:94)
      ⚬ kotlin.collections.CollectionsKt___CollectionsKt.toCollection(_Collections.kt:1155)
      ⚬ kotlin.collections.CollectionsKt___CollectionsKt.toMutableList(_Collections.kt:1188)
      ⚬ kotlin.collections.CollectionsKt___CollectionsKt.toList(_Collections.kt:1179)
      ⚬ ch.tutteli.atrium.domain.robstoll.lib.creating.IterableAssertionsKt$_iterableAll$1.invoke(iterableAssertions.kt:40)
      ⚬ ch.tutteli.atrium.domain.robstoll.lib.creating.IterableAssertionsKt$_iterableAll$1.invoke(iterableAssertions.kt)
      ⚬ ...
    at ch.tutteli.atrium.reporting.AtriumErrorKt.createAtriumError(AtriumError.kt:26)
    at ch.tutteli.atrium.reporting.AtriumError$Companion.create(AtriumError.kt:22)
    at ch.tutteli.atrium.core.robstoll.lib.checking.ThrowingAssertionChecker.check(ThrowingAssertionChecker.kt:43)
    at ch.tutteli.atrium.creating.ReportingAssertionContainer$EagerCommonFields.check(ReportingAssertionContainer.kt:111)
    at ch.tutteli.atrium.core.robstoll.lib.creating.ReportingAssertionContainerImpl.checkAndClearAssertions(ReportingAssertionContainerImpl.kt:35)
    at ch.tutteli.atrium.core.robstoll.lib.creating.ReportingAssertionContainerImpl.addAssertion(ReportingAssertionContainerImpl.kt:29)
    at ch.tutteli.atrium.core.robstoll.lib.creating.ReportingAssertionContainerImpl.addAssertion(ReportingAssertionContainerImpl.kt:11)
    at ch.tutteli.atrium.core.robstoll.lib.checking.DelegatingAssertionChecker.check(DelegatingAssertionChecker.kt:13)
    at ch.tutteli.atrium.creating.ReportingAssertionContainer$EagerCommonFields.check(ReportingAssertionContainer.kt:111)
    at ch.tutteli.atrium.core.robstoll.lib.creating.ReportingAssertionContainerImpl.checkAndClearAssertions(ReportingAssertionContainerImpl.kt:35)
    at ch.tutteli.atrium.core.robstoll.lib.creating.ReportingAssertionContainerImpl.addAssertion(ReportingAssertionContainerImpl.kt:29)
    at ch.tutteli.atrium.domain.robstoll.lib.creating.changers.ChangeSubjectKt._changeSubject(changeSubject.kt:54)
    at ch.tutteli.atrium.domain.robstoll.creating.changers.SubjectChangerImpl.reported(SubjectChangerImpl.kt:31)
    at ch.tutteli.atrium.domain.builders.creating.changers.impl.subjectchanger.FinalStepImpl.transformIt(defaultImpls.kt:75)
    at ch.tutteli.atrium.domain.builders.creating.changers.impl.subjectchanger.FinalStepImpl.access$transformIt(defaultImpls.kt:62)
    at ch.tutteli.atrium.domain.builders.creating.changers.impl.subjectchanger.FinalStepImpl$build$2.invoke(defaultImpls.kt:71)
    at ch.tutteli.atrium.domain.builders.creating.changers.impl.subjectchanger.FinalStepImpl$build$2.invoke(defaultImpls.kt:62)
    at ch.tutteli.atrium.domain.creating.changers.PostFinalStep.addToFeature(PostFinalStep.kt:51)
    at ch.tutteli.atrium.specs.AssertionCreatorSpec$1$1$$special$$inlined$forEach$lambda$1$1.invoke(AssertionCreatorSpec.kt:53)
    at ch.tutteli.atrium.specs.AssertionCreatorSpec$1$1$$special$$inlined$forEach$lambda$1$1.invoke(AssertionCreatorSpec.kt:13)
    at org.spekframework.spek2.runtime.scope.TestScopeImpl.execute(Scopes.kt:94)
    at org.spekframework.spek2.runtime.Executor$execute$$inlined$executeSafely$lambda$1$1.invokeSuspend(Executor.kt:52)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(Dispatched.kt:238)
    at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:594)
    at kotlinx.coroutines.scheduling.CoroutineScheduler.access$runSafely(CoroutineScheduler.kt:60)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:742)
aljacinto picture aljacinto on 16 Oct 2019

Those make sure that an assertion function using an assertionCreator-lambda fail if the assertionCreator is empty. For instance:

expect(listOf(1,2)).get(0) {}

Fails because one has not defined any sub-assertions for the entry at index 0.
You can fix the problem by overloading the constructor of AssertionCreatorSpec:

  • turn subject: T into a subjectProvider: () -> T
  • delegate subject: T to subjectProvider
robstoll picture robstoll on 16 Oct 2019

I thought the failure is because of the change to one time iterable (below). But it only happens in AssertionCreatorSpec

  » Properties of the unexpected IllegalStateException
    » message: "This sequence can be consumed only once."        <590669700>
    » stacktrace: 
      ⚬ kotlin.sequences.ConstrainedOnceSequence.iterator(SequencesJVM.kt:23)
      ⚬ kotlin.sequences.SequencesKt___SequencesKt$asIterable$$inlined$Iterable$1.iterator(Iterables.kt:94)
      ⚬ kotlin.collections.CollectionsKt___CollectionsKt.toCollection(_Collections.kt:1155)
      ⚬ kotlin.collections.Coll
aljacinto picture aljacinto on 17 Oct 2019

Have a look into AssertionCreatorSpec. It expects subject and consumes it multiple times => the spec not the assertion functions. Hence it is a problem of the spec and not of the implementation. If you change subject to subjectProvider: () -> T as suggested and use it in line 26 expect(subjectProvider()) then you have resolved this problem. It is basically the same problematic as for expect(oneToFour) vs. expect(oneToFour())

robstoll picture robstoll on 17 Oct 2019

That was not it, I changed as you suggested and still have the same issue. It is failing because of this chained statement:

                        expect(subject())
                            .createAssertionOk()
                            .createAssertionFail()

which is the same as doing below, which also failed because we are trying to access the same sequence on the second all call. Sorry, I don't know enough if that syntax makes sense but it is allowed.

            context("chained all") {
                it("should fail with sequence") {
                    expect(oneToSeven()).all {isGreaterThan(0.5)}.all { isGreaterThan(0.5) }
                }
            }

I can simply change the AssertionCreatorSpec to below to fix issue, let me know.

                        expect(subject())
                            .createAssertionOk()
                        expect(subject())
                            .createAssertionFail()
aljacinto picture aljacinto on 18 Oct 2019

Good. The syntax makes sense and I see that you understand the problem :+1:
In this case we have to workaround the problem differently as the test wants to assert that the report contains only the problem about the empty assertionCreator-lambda and not the createAssertionOk.
Hence:

  • change the implementation back to subject (and not subjectProvider)
  • rewrite 
 include(object : AssertionCreatorSpec<Iterable<Double>>(
        describePrefix, oneToSeven(),
        all.forAssertionCreatorSpec("$isGreaterThanDescr: 0.0") { isGreaterThan(0.0) }
    ) {})
    include(object : AssertionCreatorSpec<Iterable<Double?>>(
        "$describePrefix[nullable Element] ", oneToSeven(),
        allNullable.forAssertionCreatorSpec("$isGreaterThanDescr: 0.0") { isGreaterThan(0.0) }
    ) {})

to oneToSeven().toList().asIterable => it's OK if we use a multiple-time-consumable iterable for this test

robstoll picture robstoll on 18 Oct 2019

@robstoll Any thoughts on why it is failing in IterableAnyAssertionsSpec???

aljacinto picture aljacinto on 21 Oct 2019

IterableNoneAssertionsSpec and IterableContainsNotValuesAssertionsSpecare failing as well (and others). It could be that the implementation is wrong and you have now successfully revealed a bug I feared was in Atrium :tada:

You can either:


Dig into the stacktrace, debug the corresponding Spec and see why the iterable gets consumed twice. For instance: 

ch.tutteli.atrium.api.fluent.en_GB.IterableContainsInAnyOrderAtMostValuesAssertionsSpec > contains 5.0, 3.1, 3.0, 4.0 atMost twice throws AssertionError FAILED

Fails due to:

kotlin.sequences.ConstrainedOnceSequence.iterator(SequencesJVM.kt:23)

          ⚬ kotlin.sequences.SequencesKt___SequencesKt$asIterable$$inlined$Iterable$1.iterator(Iterables.kt:94)

          ⚬ ch.tutteli.atrium.domain.robstoll.lib.creating.iterable.contains.creators.InAnyOrderValuesAssertionCreator.search(InAnyOrderValuesAssertionCreator.kt:65)

          ⚬ ch.tutteli.atrium.domain.robstoll.lib.creating.basic.contains.creators.ContainsObjectsAssertionCreator.searchAndCreateAssertion(ContainsObjectsAssertionCreator.kt:39)

          ⚬ ch.tutteli.atrium.domain.robstoll.lib.creating.basic.contains.creators.ContainsAssertionCreator$createAssertionGroup$$inlined$map$lambda$1.invoke(ContainsAssertionCreator.kt:34)

          ⚬ ch.tutteli.atrium.domain.robstoll.lib.creating.basic.contains.creators.ContainsAssertionCreator$createAssertionGroup$$inlined$map$lambda$1.invoke(ContainsAssertionCreator.kt:24)

So set a breakpoint in InAnyOrderValuesAssertionCreator.search and analyse to understand why it consumes it twice

Or you can try to implement a quick fix by changing the subject in ch/tutteli/atrium/domain/robstoll/lib/creating/iterable/contains/creators/creators.kt => createAssertionGroup and createAssertionGroupWithoutChecker via ExpectImpl.changeSubject.unreported

robstoll picture robstoll on 21 Oct 2019

Is there a way to get rid of the coroutine timeout?

Timed out waiting for 10000 ms
kotlinx.coroutines.TimeoutCancellationException: Timed out waiting for 10000 ms
aljacinto picture aljacinto on 26 Oct 2019

I don't know it by heart but look at spekframework.org. I think it is written somewhere. Where do you get it? This should not happen. PathAssertionSpec?

robstoll picture robstoll on 26 Oct 2019

@aljacinto do you need help?

robstoll picture robstoll on 6 Nov 2019

@robstoll got sidetrack with other projects, I'll come on it tonight and let you know.

aljacinto picture aljacinto on 7 Nov 2019
👍1

@robstoll the second call is coming from this method, specifically the parameterObject.assertionFilter(assertion) . Is it suppose to call that? Put the call stack below as well for the second call to search

    private fun noNeedToFormat(assertion: Assertion, parameterObject: AssertionFormatterParameterObject): Boolean {
        //assertionFilter only applies if:
        // - we are not in an assertion group which should not be filtered (e.g. explanatory or summary group) and
        // - if the given assertion is not an explanatory assertion group either.
        return parameterObject.isNotInDoNotFilterGroup()
            && !isExplanatoryAssertionGroup(assertion)
            && !parameterObject.assertionFilter(assertion)
    }

search:48, InAnyOrderValuesAssertionCreator (ch.tutteli.atrium.domain.robstoll.lib.creating.iterable.contains.creators)
searchAndCreateAssertion:39, ContainsObjectsAssertionCreator (ch.tutteli.atrium.domain.robstoll.lib.creating.basic.contains.creators)
invoke:34, ContainsAssertionCreator$createAssertionGroup$$inlined$map$lambda$1 (ch.tutteli.atrium.domain.robstoll.lib.creating.basic.contains.creators)
invoke:13, LazyThreadUnsafeAssertionGroup$assertionGroup$2 (ch.tutteli.atrium.domain.robstoll.lib.assertions)
getAssertionGroup:-1, LazyThreadUnsafeAssertionGroup (ch.tutteli.atrium.domain.robstoll.lib.assertions)
holds:20, LazyThreadUnsafeAssertionGroup (ch.tutteli.atrium.domain.robstoll.lib.assertions)
holds:52, AssertionGroup$DefaultImpls (ch.tutteli.atrium.assertions)
holds:52, AssertionGroup$DefaultImpls (ch.tutteli.atrium.assertions)
assertionFilter:30, OnlyFailureReporter (ch.tutteli.atrium.core.robstoll.lib.reporting)
noNeedToFormat:38, AssertionFormatterControllerImpl (ch.tutteli.atrium.core.robstoll.lib.reporting)
format:20, AssertionFormatterControllerImpl (ch.tutteli.atrium.core.robstoll.lib.reporting)
format:21, AssertionFormatterControllerBasedFacade (ch.tutteli.atrium.core.robstoll.lib.reporting)
format:28, OnlyFailureReporter (ch.tutteli.atrium.core.robstoll.lib.reporting)
check:41, ThrowingAssertionChecker (ch.tutteli.atrium.core.robstoll.lib.checking)
check:111, ReportingAssertionContainer$EagerCommonFields (ch.tutteli.atrium.creating)
checkAndClearAssertions:35, ReportingAssertionContainerImpl (ch.tutteli.atrium.core.robstoll.lib.creating)
addAssertion:29, ReportingAssertionContainerImpl (ch.tutteli.atrium.core.robstoll.lib.creating)
addAssertion:41, CheckOptionExtensionKt (ch.tutteli.atrium.domain.builders.creating.basic.contains)
addAssertion:21, CheckOptionExtensionKt (ch.tutteli.atrium.domain.builders.creating.basic.contains)
values:47, IterableContainsInAnyOrderCreatorsKt (ch.tutteli.atrium.api.fluent.en_GB)
containsAtMost:20, IterableContainsInAnyOrderAtMostValuesAssertionsSpec$Companion (ch.tutteli.atrium.api.fluent.en_GB)
invoke:39, IterableContainsInAnyOrderAtMostValuesAssertionSpec$1$3 (ch.tutteli.atrium.specs.integration)
invoke:59, IterableContainsInAnyOrderAtMostValuesAssertionSpec$1$4$1$1$1 (ch.tutteli.atrium.specs.integration)
aljacinto picture aljacinto on 9 Nov 2019

@aljacinto thanks for the further investigation. This call is correct and can most likely not be changed (without a lot of effort). I suggest we implement the fix I mentioned above. Shall I take over from here?

robstoll picture robstoll on 11 Nov 2019

Go for it, I just don't know where to make that change :)

aljacinto picture aljacinto on 11 Nov 2019

Implemented with 022c9275

robstoll picture robstoll on 13 Nov 2019
Was this page helpful?
0 / 5 - 0 ratings

Related issues

robstoll picture robstoll  Â·  6Comments
robstoll picture robstoll  Â·  3Comments
robstoll picture robstoll  Â·  4Comments
dalewking picture dalewking  Â·  6Comments
robstoll picture robstoll  Â·  5Comments