[GitHub] [geode] PurelyApplied opened pull request #2549: GEODE-5803: Set test facets to inherit 'test' configuration

["PurelyApplied (GitHub)" <gi...@apache.org>]

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

- [x] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?

- [x] Is your initial contribution a single, squashed commit?

- [x] Does `gradlew build` run cleanly?

- [n/a] Have you written or updated unit tests to verify your changes?

- [n/a] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.


[ Full content available at: https://github.com/apache/geode/pull/2549 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on pull request #2549: GEODE-5803: Set test facets to inherit 'test' configuration

["PurelyApplied (GitHub)" <gi...@apache.org>]

Ahh.  Good to know.  I was thrown off by our apparent desire to have that structure as explicitly called out in `geode-core`'s build.

So would you say that this block should be considered and anti-pattern, and this ticket should be updated to address this issue instead?

```
// geode-core/build.gradle:217-223
dependencies {
  // Integration Tests
  integrationTestCompile sourceSets.test.output

  // Distributed Tests
  distributedTestCompile sourceSets.integrationTest.output
}
```

[ Full content available at: https://github.com/apache/geode/pull/2549 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied closed pull request #2549: GEODE-5803: Set test facets to inherit 'test' configuration

["PurelyApplied (GitHub)" <gi...@apache.org>]

[ pull request closed by PurelyApplied ]

[ Full content available at: https://github.com/apache/geode/pull/2549 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett commented on issue #2549: GEODE-5803: Set test facets to inherit 'test' configuration

["pivotal-jbarrett (GitHub)" <gi...@apache.org>]

It should be pretty trivial. Having been part of most of the initial refactoring I can tell you this was simply an omission.

[ Full content available at: https://github.com/apache/geode/pull/2549 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on issue #2549: GEODE-5803: Set test facets to inherit 'test' configuration

["PurelyApplied (GitHub)" <gi...@apache.org>]

This PR is based off #2547, since that PR also addresses the removal of some inherited test configuration.

[ Full content available at: https://github.com/apache/geode/pull/2549 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett commented on pull request #2549: GEODE-5803: Set test facets to inherit 'test' configuration

["pivotal-jbarrett (GitHub)" <gi...@apache.org>]

Yes this is a mistake and should have been cleaned up months ago when we cleaned up all the other sub projects. Whoops!

[ Full content available at: https://github.com/apache/geode/pull/2549 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on issue #2549: GEODE-5803: Set test facets to inherit 'test' configuration

["PurelyApplied (GitHub)" <gi...@apache.org>]

Looking at disentangling the `geode-core` source sets as mentioned above, this appears to be significantly non-trivial (and perhaps why it wasn't done during the previous test restructure).  Closing the PR until more work can be done on that front.

[ Full content available at: https://github.com/apache/geode/pull/2549 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett commented on pull request #2549: GEODE-5803: Set test facets to inherit 'test' configuration

["pivotal-jbarrett (GitHub)" <gi...@apache.org>]

NO!!!! REVERT!

This causes all sorts of bad in IJ and Eclipse and falls outside the standard model for test sources. XTest sources should not depend on the outputs of other xTest sources.
 

[ Full content available at: https://github.com/apache/geode/pull/2549 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org