You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by galen-pivotal <gi...@git.apache.org> on 2016/12/08 17:17:15 UTC
[GitHub] geode pull request #305: [GEODE-1923] Fix a test race condition.
GitHub user galen-pivotal opened a pull request:
https://github.com/apache/geode/pull/305
[GEODE-1923] Fix a test race condition.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/galen-pivotal/incubator-geode feature/GEODE-1923
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/geode/pull/305.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #305
----
commit 8373c0d9152bfe52fc8a9094e9a6e316aa498e9f
Author: Galen O'Sullivan <go...@pivotal.io>
Date: 2016-12-07T20:23:46Z
[GEODE-1923] Fix a test race condition.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #305: [GEODE-1923] Fix a test race condition.
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/305#discussion_r94657673
--- Diff: geode-core/src/test/java/org/apache/geode/internal/cache/FixedPRSinglehopDUnitTest.java ---
@@ -293,18 +294,23 @@ public void test_FPAmetadataFetch() {
putIntoPartitionedRegionsThreeQs();
getFromPartitionedRegionsFor3Qs();
- Wait.pause(2000);
+ // Server 1 is actually primary for both Q1 and Q2, since there is no FPA server with
+ // primary set to true.
+ Awaitility.await().atMost(15, TimeUnit.SECONDS).until(
+ () -> (server1.invoke(() -> FixedPRSinglehopDUnitTest.primaryBucketsOnServer()) == 6)
--- End diff --
I like this style better, but it conflicts with the style of the rest of the test. I'm inclined to favor consistent style over nicer style.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #305: [GEODE-1923] Fix a test race condition.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/geode/pull/305
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #305: [GEODE-1923] Fix a test race condition.
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/305#discussion_r94660604
--- Diff: geode-core/src/test/java/org/apache/geode/internal/cache/FixedPRSinglehopDUnitTest.java ---
@@ -293,18 +294,23 @@ public void test_FPAmetadataFetch() {
putIntoPartitionedRegionsThreeQs();
getFromPartitionedRegionsFor3Qs();
- Wait.pause(2000);
+ // Server 1 is actually primary for both Q1 and Q2, since there is no FPA server with
+ // primary set to true.
+ Awaitility.await().atMost(15, TimeUnit.SECONDS).until(
+ () -> (server1.invoke(() -> FixedPRSinglehopDUnitTest.primaryBucketsOnServer()) == 6)
--- End diff --
Actually scratch that, I'm going to change to the style you suggested. \U0001f44d
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #305: [GEODE-1923] Fix a test race condition.
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on the issue:
https://github.com/apache/geode/pull/305
@kohlmu-pivotal I just think it's a bit cleaner syntactically; there's no other real benefit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #305: [GEODE-1923] Fix a test race condition.
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on the issue:
https://github.com/apache/geode/pull/305
@kohlmu-pivotal Could you merge this for me please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #305: [GEODE-1923] Fix a test race condition.
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/305#discussion_r91569800
--- Diff: geode-core/src/test/java/org/apache/geode/internal/cache/FixedPRSinglehopDUnitTest.java ---
@@ -293,18 +295,24 @@ public void test_FPAmetadataFetch() {
putIntoPartitionedRegionsThreeQs();
getFromPartitionedRegionsFor3Qs();
- Wait.pause(2000);
+ Awaitility.await().atMost(15, TimeUnit.SECONDS).until(() ->
+ // Server 1 is actually primary for both Q1 and Q2, since there is no FPA server with
--- End diff --
@bschuchardt `./gradlew spotlessApply` wants to reduce the indent on this comment and the following lines to the same as the previous line... looks like a bug with Spotless. I'll just move the comment above the wait.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #305: [GEODE-1923] Fix a test race condition.
Posted by jaredjstewart <gi...@git.apache.org>.
Github user jaredjstewart commented on a diff in the pull request:
https://github.com/apache/geode/pull/305#discussion_r92525359
--- Diff: geode-core/src/test/java/org/apache/geode/internal/cache/FixedPRSinglehopDUnitTest.java ---
@@ -293,18 +294,23 @@ public void test_FPAmetadataFetch() {
putIntoPartitionedRegionsThreeQs();
getFromPartitionedRegionsFor3Qs();
- Wait.pause(2000);
+ // Server 1 is actually primary for both Q1 and Q2, since there is no FPA server with
+ // primary set to true.
+ Awaitility.await().atMost(15, TimeUnit.SECONDS).until(
+ () -> (server1.invoke(() -> FixedPRSinglehopDUnitTest.primaryBucketsOnServer()) == 6)
--- End diff --
I think you can simplify the lambda expression passed to `server1.invoke()` by using a method reference instead:
```
() -> (server1.invoke(FixedPRSinglehopDUnitTest::primaryBucketsOnServer) == 6)
```
If `primaryBucketsOnServer` was not a static method, it would look even shorter:
```
() -> (server1.invoke(this::primaryBucketsOnServer) == 6)
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #305: [GEODE-1923] Fix a test race condition.
Posted by kohlmu-pivotal <gi...@git.apache.org>.
Github user kohlmu-pivotal commented on the issue:
https://github.com/apache/geode/pull/305
I doubt that FixedPRSinglehopDUnitTest::primaryBucketsOnServer provides any benefit over FixedPRSinglehopDUnitTest.primaryBucketsOnServer()
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #305: [GEODE-1923] Fix a test race condition.
Posted by bschuchardt <gi...@git.apache.org>.
Github user bschuchardt commented on the issue:
https://github.com/apache/geode/pull/305
It looks like you also need to run "./gradlew spotlessApply" to reformat your changes. The Travis build complained that spotlessJavaCheck failed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---