You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@rya.apache.org by ejwhite922 <gi...@git.apache.org> on 2017/06/22 15:39:11 UTC
[GitHub] incubator-rya pull request #173: RYA-304 Fixed rya.merger integration tests
GitHub user ejwhite922 opened a pull request:
https://github.com/apache/incubator-rya/pull/173
RYA-304 Fixed rya.merger integration tests
## Description
rya.merger integration tests were failing and getting incorrect values for the expected query result counts. Making sure inferencing was passed on to the sail config fixed this.
### Tests
Unit Tests
### Links
[Jira](https://issues.apache.org/jira/browse/RYA-304)
### Checklist
- [ ] Code Review
- [ ] Squash Commits
#### People To Review
@isper3at
@DLotts
@meiercaleb
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ejwhite922/incubator-rya RYA-304_FixRyaMergerIntegrationTests
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-rya/pull/173.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 #173
----
commit 4b3a280a1e9c49e9a7c4accc3f822c7361934e3c
Author: eric.white <er...@parsons.com>
Date: 2017-06-22T15:35:03Z
RYA-304 Fixed rya.merger integration tests
----
---
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] incubator-rya pull request #173: RYA-304 Fixed rya.merger integration tests
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-rya/pull/173
---
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] incubator-rya pull request #173: RYA-304 Fixed rya.merger integration tests
Posted by ejwhite922 <gi...@git.apache.org>.
Github user ejwhite922 commented on a diff in the pull request:
https://github.com/apache/incubator-rya/pull/173#discussion_r123565355
--- Diff: extras/rya.merger/src/test/java/org/apache/rya/accumulo/mr/merge/RulesetCopyIT.java ---
@@ -144,32 +146,33 @@ private static RyaType literal(final String lit, final URI type) {
@BeforeClass
public static void setUpPerClass() throws Exception {
+ DemoUtilities.setupLogging(LoggingDetail.LIGHT);
accumuloDualInstanceDriver = new AccumuloDualInstanceDriver(IS_MOCK, true, true, false, false);
accumuloDualInstanceDriver.setUpInstances();
}
@Before
public void setUpPerTest() throws Exception {
- parentConfig = accumuloDualInstanceDriver.getParentConfig();
- childConfig = accumuloDualInstanceDriver.getChildConfig();
- accumuloDualInstanceDriver.setUpTables();
accumuloDualInstanceDriver.setUpConfigs();
+ accumuloDualInstanceDriver.setUpTables();
accumuloDualInstanceDriver.setUpDaos();
+ parentConfig = accumuloDualInstanceDriver.getParentConfig();
+ childConfig = accumuloDualInstanceDriver.getChildConfig();
parentDao = accumuloDualInstanceDriver.getParentDao();
}
@After
public void tearDownPerTest() throws Exception {
log.info("tearDownPerTest(): tearing down now.");
- accumuloDualInstanceDriver.tearDownDaos();
accumuloDualInstanceDriver.tearDownTables();
+ accumuloDualInstanceDriver.tearDownDaos();
+ if (rulesetTool != null) {
+ rulesetTool.shutdown();
+ }
}
@AfterClass
public static void tearDownPerClass() throws Exception {
- if (rulesetTool != null) {
- rulesetTool.shutdown();
- }
accumuloDualInstanceDriver.tearDown();
--- End diff --
This way caused the tests to run faster and were a bit more reliable. Trying to spin up a new MAC for each test and tear it down nearly doubled the time for running the tests. It looked like more java processes were opened by spinning up MACs for each test.
---
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] incubator-rya pull request #173: RYA-304 Fixed rya.merger integration tests
Posted by jessehatfield <gi...@git.apache.org>.
Github user jessehatfield commented on a diff in the pull request:
https://github.com/apache/incubator-rya/pull/173#discussion_r123563097
--- Diff: extras/rya.merger/src/test/java/org/apache/rya/accumulo/mr/merge/RulesetCopyIT.java ---
@@ -144,32 +146,33 @@ private static RyaType literal(final String lit, final URI type) {
@BeforeClass
public static void setUpPerClass() throws Exception {
+ DemoUtilities.setupLogging(LoggingDetail.LIGHT);
accumuloDualInstanceDriver = new AccumuloDualInstanceDriver(IS_MOCK, true, true, false, false);
accumuloDualInstanceDriver.setUpInstances();
}
@Before
public void setUpPerTest() throws Exception {
- parentConfig = accumuloDualInstanceDriver.getParentConfig();
- childConfig = accumuloDualInstanceDriver.getChildConfig();
- accumuloDualInstanceDriver.setUpTables();
accumuloDualInstanceDriver.setUpConfigs();
+ accumuloDualInstanceDriver.setUpTables();
accumuloDualInstanceDriver.setUpDaos();
+ parentConfig = accumuloDualInstanceDriver.getParentConfig();
+ childConfig = accumuloDualInstanceDriver.getChildConfig();
parentDao = accumuloDualInstanceDriver.getParentDao();
}
@After
public void tearDownPerTest() throws Exception {
log.info("tearDownPerTest(): tearing down now.");
- accumuloDualInstanceDriver.tearDownDaos();
accumuloDualInstanceDriver.tearDownTables();
+ accumuloDualInstanceDriver.tearDownDaos();
+ if (rulesetTool != null) {
+ rulesetTool.shutdown();
+ }
}
@AfterClass
public static void tearDownPerClass() throws Exception {
- if (rulesetTool != null) {
- rulesetTool.shutdown();
- }
accumuloDualInstanceDriver.tearDown();
--- End diff --
I can verify that it this helps. Ran tests from the command line ("mvn verify"), monitored the number of org.apache.accumulo.start.Main processes running: seems to vary between 11 (during tests) down to 5 (between tests), where previously it continued to climb up to a max of 31 for me. And the change makes sense: the tool gets initialized in each test (initialization happens in runRulesetCopyTest, which each test calls), so it should be torn down after each test.
---
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] incubator-rya pull request #173: RYA-304 Fixed rya.merger integration tests
Posted by ejwhite922 <gi...@git.apache.org>.
Github user ejwhite922 commented on a diff in the pull request:
https://github.com/apache/incubator-rya/pull/173#discussion_r123581881
--- Diff: extras/rya.merger/src/test/java/org/apache/rya/accumulo/mr/merge/RulesetCopyIT.java ---
@@ -144,32 +146,33 @@ private static RyaType literal(final String lit, final URI type) {
@BeforeClass
public static void setUpPerClass() throws Exception {
+ DemoUtilities.setupLogging(LoggingDetail.LIGHT);
accumuloDualInstanceDriver = new AccumuloDualInstanceDriver(IS_MOCK, true, true, false, false);
accumuloDualInstanceDriver.setUpInstances();
}
@Before
public void setUpPerTest() throws Exception {
- parentConfig = accumuloDualInstanceDriver.getParentConfig();
- childConfig = accumuloDualInstanceDriver.getChildConfig();
- accumuloDualInstanceDriver.setUpTables();
accumuloDualInstanceDriver.setUpConfigs();
+ accumuloDualInstanceDriver.setUpTables();
accumuloDualInstanceDriver.setUpDaos();
+ parentConfig = accumuloDualInstanceDriver.getParentConfig();
+ childConfig = accumuloDualInstanceDriver.getChildConfig();
parentDao = accumuloDualInstanceDriver.getParentDao();
}
@After
public void tearDownPerTest() throws Exception {
log.info("tearDownPerTest(): tearing down now.");
- accumuloDualInstanceDriver.tearDownDaos();
accumuloDualInstanceDriver.tearDownTables();
+ accumuloDualInstanceDriver.tearDownDaos();
+ if (rulesetTool != null) {
+ rulesetTool.shutdown();
+ }
}
@AfterClass
public static void tearDownPerClass() throws Exception {
- if (rulesetTool != null) {
- rulesetTool.shutdown();
- }
accumuloDualInstanceDriver.tearDown();
--- End diff --
After checking with everyone, I think we're now good with my changes since they teardown the tool's newly created child instance after each test like Andrew's changes. My PR just keeps the same parent MAC instance up for the class and tears it down at the end of the class.
---
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] incubator-rya pull request #173: RYA-304 Fixed rya.merger integration tests
Posted by isper3at <gi...@git.apache.org>.
Github user isper3at commented on a diff in the pull request:
https://github.com/apache/incubator-rya/pull/173#discussion_r123551954
--- Diff: extras/rya.merger/src/test/java/org/apache/rya/accumulo/mr/merge/RulesetCopyIT.java ---
@@ -144,32 +146,33 @@ private static RyaType literal(final String lit, final URI type) {
@BeforeClass
public static void setUpPerClass() throws Exception {
+ DemoUtilities.setupLogging(LoggingDetail.LIGHT);
accumuloDualInstanceDriver = new AccumuloDualInstanceDriver(IS_MOCK, true, true, false, false);
accumuloDualInstanceDriver.setUpInstances();
}
@Before
public void setUpPerTest() throws Exception {
- parentConfig = accumuloDualInstanceDriver.getParentConfig();
- childConfig = accumuloDualInstanceDriver.getChildConfig();
- accumuloDualInstanceDriver.setUpTables();
accumuloDualInstanceDriver.setUpConfigs();
+ accumuloDualInstanceDriver.setUpTables();
accumuloDualInstanceDriver.setUpDaos();
+ parentConfig = accumuloDualInstanceDriver.getParentConfig();
+ childConfig = accumuloDualInstanceDriver.getChildConfig();
parentDao = accumuloDualInstanceDriver.getParentDao();
}
@After
public void tearDownPerTest() throws Exception {
log.info("tearDownPerTest(): tearing down now.");
- accumuloDualInstanceDriver.tearDownDaos();
accumuloDualInstanceDriver.tearDownTables();
+ accumuloDualInstanceDriver.tearDownDaos();
+ if (rulesetTool != null) {
+ rulesetTool.shutdown();
+ }
}
@AfterClass
public static void tearDownPerClass() throws Exception {
- if (rulesetTool != null) {
- rulesetTool.shutdown();
- }
accumuloDualInstanceDriver.tearDown();
--- End diff --
so I moved all the tearDowns() into the '@'after because miniAccumulos were being left open between tests. Can you verify that it isn't happening? Otherwise, doing a full teardown between tests did not add a lot of time to the tests.
---
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] incubator-rya pull request #173: RYA-304 Fixed rya.merger integration tests
Posted by jessehatfield <gi...@git.apache.org>.
Github user jessehatfield commented on a diff in the pull request:
https://github.com/apache/incubator-rya/pull/173#discussion_r123583040
--- Diff: extras/rya.merger/src/test/java/org/apache/rya/accumulo/mr/merge/RulesetCopyIT.java ---
@@ -144,32 +146,33 @@ private static RyaType literal(final String lit, final URI type) {
@BeforeClass
public static void setUpPerClass() throws Exception {
+ DemoUtilities.setupLogging(LoggingDetail.LIGHT);
accumuloDualInstanceDriver = new AccumuloDualInstanceDriver(IS_MOCK, true, true, false, false);
accumuloDualInstanceDriver.setUpInstances();
}
@Before
public void setUpPerTest() throws Exception {
- parentConfig = accumuloDualInstanceDriver.getParentConfig();
- childConfig = accumuloDualInstanceDriver.getChildConfig();
- accumuloDualInstanceDriver.setUpTables();
accumuloDualInstanceDriver.setUpConfigs();
+ accumuloDualInstanceDriver.setUpTables();
accumuloDualInstanceDriver.setUpDaos();
+ parentConfig = accumuloDualInstanceDriver.getParentConfig();
+ childConfig = accumuloDualInstanceDriver.getChildConfig();
parentDao = accumuloDualInstanceDriver.getParentDao();
}
@After
public void tearDownPerTest() throws Exception {
log.info("tearDownPerTest(): tearing down now.");
- accumuloDualInstanceDriver.tearDownDaos();
accumuloDualInstanceDriver.tearDownTables();
+ accumuloDualInstanceDriver.tearDownDaos();
+ if (rulesetTool != null) {
+ rulesetTool.shutdown();
+ }
}
@AfterClass
public static void tearDownPerClass() throws Exception {
- if (rulesetTool != null) {
- rulesetTool.shutdown();
- }
accumuloDualInstanceDriver.tearDown();
--- End diff --
Oh, I suppose there are two parts: The AccumuloDualInstanceDriver is initialized once per class, and in turn initializes the parent MAC. But the child MAC is instead initialized when the tool is run (via CopyTool.createChildInstance), which is once per test. Calling rulesetTool.shutdown() will close the child instance (but not the parent). Previously, this shutdown call was only being made once per class, so the extra child MACs weren't being shut down between tests. This PR fixes that by moving that call into the per-test shutdown. This leaves the parent MAC open between tests (hence the minimum 5 processes), but there's only ever one parent MAC (the one that was initialized by the AccumuloDualInstanceDriver) and it does get shut down at the end, so we don't have these extraneous processes. So I believe this PR solves the problem.
---
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] incubator-rya issue #173: RYA-304 Fixed rya.merger integration tests
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-rya/pull/173
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/244/
---
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.
---