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.

Sign up for GitHub

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 timing issue in Elasticsearch tests #481

Merged
merged 3 commits into from
Sep 25, 2017
Merged

Fix timing issue in Elasticsearch tests #481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
20839e9
Disable parallel execution in elasticsearch project
takezoe Sep 19, 2017
cf9084f
Using different port in Java and Scala test is unnecessary any more
takezoe Sep 20, 2017
77cc8ad
Add comment to explain reason of disabling parallel test execution
takezoe Sep 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ lazy val elasticsearch = project
.enablePlugins(AutomateHeaderPlugin)
.settings(
name := "akka-stream-alpakka-elasticsearch",
Dependencies.Elasticsearch
Dependencies.Elasticsearch,
// For elasticsearch-cluster-runner https://github.com/akka/alpakka/issues/479
parallelExecution in Test := false
)

lazy val files = project // The name file is taken by `sbt.file`!
Expand Down
7 changes: 5 additions & 2 deletions elasticsearch/src/test/java/akka/stream/alpakka/elasticsearch/ElasticsearchTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,14 @@ public static class Book {
@BeforeClass
public static void setup() throws IOException {
runner = new ElasticsearchClusterRunner();
runner.build(ElasticsearchClusterRunner.newConfigs().baseHttpPort(9210).baseTransportPort(9310).numOfNode(1));
runner.build(ElasticsearchClusterRunner.newConfigs()
.baseHttpPort(9200)
.baseTransportPort(9300)
.numOfNode(1));
runner.ensureYellow();

//#init-client
client = RestClient.builder(new HttpHost("localhost", 9211)).build();
client = RestClient.builder(new HttpHost("localhost", 9201)).build();
Copy link
Contributor

Choose a reason for hiding this comment

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

When (intentionally) re-using the same port, you could consider putting the actual port number in a constant somewhere and referring to it from both places where you use it.

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 sharing the port number is not necessary. These tests are independent and they don't have to use same port necessarily. It's just no longer necessary to use different ports.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, using a constant make it more obvious that it's intentional though. But OK like this 👍

//#init-client

//#init-mat
Expand Down
8 changes: 7 additions & 1 deletion elasticsearch/src/test/scala/akka/stream/alpakka/elasticsearch/ElasticsearchSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ class ElasticsearchSpec extends WordSpec with Matchers with BeforeAndAfterAll {
//#define-class

override def beforeAll() = {
runner.build(ElasticsearchClusterRunner.newConfigs().baseHttpPort(9200).baseTransportPort(9300).numOfNode(1))
runner.build(
ElasticsearchClusterRunner
.newConfigs()
.baseHttpPort(9200)
.baseTransportPort(9300)
.numOfNode(1)
)
runner.ensureYellow()

register("source", "Akka in Action")
Expand Down