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

[WIP] Updating kotlintest to 3.0 #786

Closed
wants to merge 5 commits into from
Closed

[WIP] Updating kotlintest to 3.0 #786

wants to merge 5 commits into from

Conversation

andrzejressel
Copy link
Contributor

@andrzejressel andrzejressel commented Apr 10, 2018

A lot of things change and we'll propably have to update it anyway in the future (for JS support), so let's do this as early as possible.

TODO:
In Specs replace Gen, with () => Gen - generator can only be used one time on 3.0

override fun always() = listOf(RuntimeException(), NoSuchElementException(), IllegalArgumentException())

override fun random(): Sequence<Throwable> {
return Gen.from(always()).random()
Copy link
Member

@pakoito pakoito Apr 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please fix the Gen.from to a field so it keeps the seed across calls? This was a bug on the previous implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main problem with 3.0 is that random returns sequence that can be used only one time. So tests will crash when random will be invoked and used second time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooooooooooh. I see. It's different now. Yeah ignore this and the other I'll comment on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess, I was wrong.

res.equalUnderTheLaw(just(iter), EQ)
}
fun <F> Monad<F>.stackSafety(iterations: Int = 5000, EQ: Eq<Kind<F, Int>>): Unit = Unit
// forFew(1, Gen.oneOf(listOf(iterations))) { iter ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any issue with these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think forFew doesn't exist anymore

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in arrow-test, I wrote it for tests that'd cause OOMs on CI :D

fun <A : Any> genNullable(genA: Gen<A>): Gen<A?> = object: Gen<A?> {
override fun always(): Iterable<A?> = emptyList()

override fun random(): Sequence<A?> = oneOf(genA, Gen.create { Option.empty<A>() }).random().map {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to capture oneOf(..) as a field, and only call random here, to not reset the seed on every call.

Can't we use Gen.create { null } on 3.X?

Copy link
Contributor Author

@andrzejressel andrzejressel Apr 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, sequences cannot have null values in stream.

Copy link
Member

@pakoito pakoito Apr 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:( Can use use another marker value? Like, a literal value we create inside the class with an identity that will not overlap with A? Maybe compare using ===?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wrong here also, sequence can have null values, but sequences generated throught generateSequence cannot (because null defines the end of the stream)

is E -> Left(it)
is A -> Right(it)
else -> throw IllegalStateException("genEither incorrect value $it")
is E -> Left(it)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting with two spaces here!

//// rootTestSuite.addTestCase(tc)
//// tc
// val tc = TestCase(Description(emptyList(), law.name),this@UnitSpec, { law.test()} , 0, defaultTestCaseConfig)
// }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete?

build.gradle Outdated
maxParallelForks = Runtime.runtime.availableProcessors()
}

// build.dependsOn ':detekt'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fiiix.

@@ -117,7 +117,7 @@ subprojects { project ->
maxParallelForks = Runtime.runtime.availableProcessors()
}

build.dependsOn ':detekt'
// build.dependsOn ':detekt'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Contributor Author

@andrzejressel andrzejressel Apr 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It throws errors for files in "generated" directory - I must finally fix it in config

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only run detekt on clean builds. We normally use assemble and test rather than build to make the lib!

@pakoito
Copy link
Member

pakoito commented Jun 27, 2018

@jereksel what happened?

@andrzejressel
Copy link
Contributor Author

It's superseded by #912, isn't it?

@pakoito
Copy link
Member

pakoito commented Jun 27, 2018

Oh, right!

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

Successfully merging this pull request may close these issues.

3 participants