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.

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

GEODE-7000: Extract reusable rules from GatewayReceiverMetricsTest #3825

Closed
wants to merge 2 commits into from

Conversation

demery-pivotal
Copy link
Contributor

@demery-pivotal demery-pivotal commented Jul 22, 2019

Extract jar-making code from GatewayReceiverMetricsTest into rules for use in other tests:

  • SingleFunctionJarRule
  • MetricsPublishingServiceJarRule

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

  • Has your PR been rebased against the latest commit within the target branch (typically develop)?

  • Is your initial contribution a single, squashed commit?

  • Does gradlew build run cleanly?

@aaronlindsey Please review

@moleske
Copy link
Member

moleske commented Jul 23, 2019

It seems like GEODE-7005 has been filed for the failing test in UnitTestOpenJDK11 (also happened on develop https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UnitTestOpenJDK8/builds/903

@aaronlindsey
Copy link

@moleske
Copy link
Member

moleske commented Jul 23, 2019

Yeah, I think the idea was to merge this first, but the other PR also got finished around the same time and takes advantage of this extraction

@aaronlindsey
Copy link

The version in the other PR is more flexible. It allows creating JARs for any class meant to be used by the service loader. It also doesn't create the JAR for you each test. You have to call createJarFor explicitly. This was done because some tests in the test class don't need the JAR.

@moleske
Copy link
Member

moleske commented Jul 23, 2019

Oh I looked at the wrong PR was looking at #3829

@mhansonp
Copy link
Contributor

So, I see some opinions expressed here. Are any of them going to be moved on or should I review this code as it stands?

@demery-pivotal
Copy link
Contributor Author

This work was concurrently done (and merged) as part of #3824, rendering this PR obsolete.

@demery-pivotal demery-pivotal deleted the GEODE-7000-extract-jar-rules branch July 29, 2019 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants