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

Fix test assertion to be called. Fix array equality test for headers. #98

Merged
merged 1 commit into from
Jan 7, 2021

Conversation

mishamo
Copy link
Contributor

@mishamo mishamo commented Jan 6, 2021

No description provided.

publish(schedules) andThen
(assertMessagesWrittenFrom(_, schedules))
publish(schedules)
.foreach(assertMessagesWrittenFrom(_, schedules))
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 andThen returns a partial function which was never actually called, so the assertion never got executed.

cr.headers().toArray.map(h => h.key() -> h.value()).toMap should contain theSameElementsAs schedule.headers
cr.headers().toArray.map(h => h.key() -> h.value().toList) should contain theSameElementsAs
schedule.headers.map {
case (k, v) => (k, v.toList)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to the values being Array[Byte], the equality test would always fail, even if they were equivalent (due to array equality basically being rubbish in scala).

@@ -8,7 +8,7 @@ import scala.concurrent.duration._

package object e2e {

val Tolerance = 200 millis
val Tolerance = 400 millis
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the assertion enabled, I found the test to be flaky with values under 400 millis

@@ -48,7 +49,10 @@ class SchedulerIntSpec extends SchedulerIntSpecBase {
cr.key should contain theSameElementsInOrderAs schedule.key
cr.value should contain theSameElementsInOrderAs schedule.value.get
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As random[ScheduledEvent] can generate the value field as None around 10% of the time, it causes the right hand side of this test to fail. Investigating a possible fix for this.

Copy link
Contributor

@lacarvalho91 lacarvalho91 left a comment

Choose a reason for hiding this comment

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

good spot, thanks!

@lacarvalho91 lacarvalho91 merged commit 6c34fff into master Jan 7, 2021
@lacarvalho91 lacarvalho91 deleted the fix-test-assertion branch January 7, 2021 11:10
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.

2 participants