Skip to content

moving pulsar storm tests under tests#3000

Merged
merlimat merged 1 commit intoapache:masterfrom
jerrypeng:move_storm_tests
Nov 16, 2018
Merged

moving pulsar storm tests under tests#3000
merlimat merged 1 commit intoapache:masterfrom
jerrypeng:move_storm_tests

Conversation

@jerrypeng
Copy link
Contributor

Motivation

Similar issue as:

#2890

but with the tests for pulsar storm. Since pulsar-broker depends on pulsar-client-original and the storm pulsar adaptor depends on the shaded version of it.

After removing the function-metrics module:

#2982

The pulsar storm tests started throwing the following error:

72086 [pulsar-timer-9-1] WARN o.a.p.c.i.ProducerImpl - [persistent://public/crawl/in-partition-1] [crawler-1-1540895529_1] error while closing out batch -- java.lang.NoSuchMethodError: com.scurrilous.circe.checksum.Crc32cIntChecksum.computeChecksum(Lio/netty/buffer/ByteBuf;)I

@jerrypeng jerrypeng requested review from merlimat and sijie November 16, 2018 21:10
@jerrypeng jerrypeng self-assigned this Nov 16, 2018
@jerrypeng jerrypeng added this to the 2.3.0 milestone Nov 16, 2018
@merlimat
Copy link
Contributor

If it works and it unblocks the other change I’m ok with this. In general, for integration tests we should start the broker from a container, rather than the embedded broker in same process.

@jerrypeng
Copy link
Contributor Author

@merlimat this really doesn't have to be an integration test. It just needs to be in a separate module

@merlimat
Copy link
Contributor

@merlimat this really doesn't have to be an integration test. It just needs to be in a separate module

I know, that's why I was saying that if it works and it unblocks the other issue, we can go with this.

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

Let's make this a "proper" integration test later on.

@merlimat merlimat merged commit 784044b into apache:master Nov 16, 2018
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