You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Kirk Lund <ki...@gmail.com> on 2016/10/04 20:23:28 UTC
Review Request 52525: GEODE-1902: add ACCEPT and DENY tests for
GEODE_VERBOSE and GEMFIRE_VERBOSE
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52525/
-----------------------------------------------------------
Review request for geode and Kevin Duling.
Bugs: GEODE-1902
https://issues.apache.org/jira/browse/GEODE-1902
Repository: geode
Description
-------
GEODE-1902: add ACCEPT and DENY tests for GEODE_VERBOSE and GEMFIRE_VERBOSE
log4j2-custom.xml isn't really sufficient for testing GEODE_VERBOSE and GEMFIRE_VERBOSE. This changeset adds in new 4 log4j2 xml files that enable and disable each of the VERBOSE markers.
* add two log4j2 xml files for GEMFIRE_VERBOSE: log4j2-gemfire_verbose-accept.xml, log4j2-gemfire_verbose-deny.xml
* add two log4j2 xml files for GEODE_VERBOSE: log4j2-geode_verbose-accept.xml, log4j2-geode_verbose-deny.xml
* rename test GeodeVerboseLogMarkerIntegrationTest
* add test methods to verify every combination of behavior for accept and deny xml files
Diffs
-----
geode-core/src/test/java/org/apache/geode/internal/logging/log4j/Configuration.java PRE-CREATION
geode-core/src/test/java/org/apache/geode/internal/logging/log4j/GeodeVerboseLogMarkerIntegrationTest.java PRE-CREATION
geode-core/src/test/java/org/apache/geode/internal/logging/log4j/LogMarkerJUnitTest.java ec9d257892b64bb6ad2fc58755a448c70ddf538d
geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-accept.xml PRE-CREATION
geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-deny.xml PRE-CREATION
geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-accept.xml PRE-CREATION
geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-deny.xml PRE-CREATION
Diff: https://reviews.apache.org/r/52525/diff/
Testing
-------
GeodeVerboseLogMarkerIntegrationTest
Thanks,
Kirk Lund
Re: Review Request 52525: GEODE-1902: add ACCEPT and DENY tests for
GEODE_VERBOSE and GEMFIRE_VERBOSE
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52525/#review151380
-----------------------------------------------------------
Ship it!
Ship It!
- Jinmei Liao
On Oct. 4, 2016, 8:23 p.m., Kirk Lund wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52525/
> -----------------------------------------------------------
>
> (Updated Oct. 4, 2016, 8:23 p.m.)
>
>
> Review request for geode and Kevin Duling.
>
>
> Bugs: GEODE-1902
> https://issues.apache.org/jira/browse/GEODE-1902
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1902: add ACCEPT and DENY tests for GEODE_VERBOSE and GEMFIRE_VERBOSE
>
> log4j2-custom.xml isn't really sufficient for testing GEODE_VERBOSE and GEMFIRE_VERBOSE. This changeset adds in new 4 log4j2 xml files that enable and disable each of the VERBOSE markers.
>
> * add two log4j2 xml files for GEMFIRE_VERBOSE: log4j2-gemfire_verbose-accept.xml, log4j2-gemfire_verbose-deny.xml
> * add two log4j2 xml files for GEODE_VERBOSE: log4j2-geode_verbose-accept.xml, log4j2-geode_verbose-deny.xml
> * rename test GeodeVerboseLogMarkerIntegrationTest
> * add test methods to verify every combination of behavior for accept and deny xml files
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/Configuration.java PRE-CREATION
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/GeodeVerboseLogMarkerIntegrationTest.java PRE-CREATION
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/LogMarkerJUnitTest.java ec9d257892b64bb6ad2fc58755a448c70ddf538d
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-accept.xml PRE-CREATION
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-deny.xml PRE-CREATION
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-accept.xml PRE-CREATION
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-deny.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52525/diff/
>
>
> Testing
> -------
>
> GeodeVerboseLogMarkerIntegrationTest
>
>
> Thanks,
>
> Kirk Lund
>
>
Re: Review Request 52525: GEODE-1902: add ACCEPT and DENY tests for
GEODE_VERBOSE and GEMFIRE_VERBOSE
Posted by Kirk Lund <ki...@gmail.com>.
> On Oct. 4, 2016, 8:57 p.m., Kevin Duling wrote:
> > Some things to consider:
> > * * Your teardown code is identical to the teardown code in CustomConfigWithLogServiceIntegrationTest.
> > * Your preassertions are repeated in createConfigFile.
> > * You've tested for ACCEPT and DENY. Is there value in testing for NEUTRAL?
> > * Are you sure you want to be using @Before and not @BeforeClass? @Before is called before each and every test. @BeforeClass is only called once. This is causing you to call preassert() eight times. Then createConfigFile() is called a total of thirty-two times.
I didn't mean to leave the preassertions in there. I added then when I had something misbehaving. I'll submit another change review in a bit.
- Kirk
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52525/#review151390
-----------------------------------------------------------
On Oct. 4, 2016, 8:23 p.m., Kirk Lund wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52525/
> -----------------------------------------------------------
>
> (Updated Oct. 4, 2016, 8:23 p.m.)
>
>
> Review request for geode and Kevin Duling.
>
>
> Bugs: GEODE-1902
> https://issues.apache.org/jira/browse/GEODE-1902
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1902: add ACCEPT and DENY tests for GEODE_VERBOSE and GEMFIRE_VERBOSE
>
> log4j2-custom.xml isn't really sufficient for testing GEODE_VERBOSE and GEMFIRE_VERBOSE. This changeset adds in new 4 log4j2 xml files that enable and disable each of the VERBOSE markers.
>
> * add two log4j2 xml files for GEMFIRE_VERBOSE: log4j2-gemfire_verbose-accept.xml, log4j2-gemfire_verbose-deny.xml
> * add two log4j2 xml files for GEODE_VERBOSE: log4j2-geode_verbose-accept.xml, log4j2-geode_verbose-deny.xml
> * rename test GeodeVerboseLogMarkerIntegrationTest
> * add test methods to verify every combination of behavior for accept and deny xml files
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/Configuration.java PRE-CREATION
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/GeodeVerboseLogMarkerIntegrationTest.java PRE-CREATION
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/LogMarkerJUnitTest.java ec9d257892b64bb6ad2fc58755a448c70ddf538d
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-accept.xml PRE-CREATION
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-deny.xml PRE-CREATION
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-accept.xml PRE-CREATION
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-deny.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52525/diff/
>
>
> Testing
> -------
>
> GeodeVerboseLogMarkerIntegrationTest
>
>
> Thanks,
>
> Kirk Lund
>
>
Re: Review Request 52525: GEODE-1902: add ACCEPT and DENY tests for
GEODE_VERBOSE and GEMFIRE_VERBOSE
Posted by Kevin Duling <kd...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52525/#review151390
-----------------------------------------------------------
Ship it!
Some things to consider:
* * Your teardown code is identical to the teardown code in CustomConfigWithLogServiceIntegrationTest.
* Your preassertions are repeated in createConfigFile.
* You've tested for ACCEPT and DENY. Is there value in testing for NEUTRAL?
* Are you sure you want to be using @Before and not @BeforeClass? @Before is called before each and every test. @BeforeClass is only called once. This is causing you to call preassert() eight times. Then createConfigFile() is called a total of thirty-two times.
- Kevin Duling
On Oct. 4, 2016, 1:23 p.m., Kirk Lund wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52525/
> -----------------------------------------------------------
>
> (Updated Oct. 4, 2016, 1:23 p.m.)
>
>
> Review request for geode and Kevin Duling.
>
>
> Bugs: GEODE-1902
> https://issues.apache.org/jira/browse/GEODE-1902
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1902: add ACCEPT and DENY tests for GEODE_VERBOSE and GEMFIRE_VERBOSE
>
> log4j2-custom.xml isn't really sufficient for testing GEODE_VERBOSE and GEMFIRE_VERBOSE. This changeset adds in new 4 log4j2 xml files that enable and disable each of the VERBOSE markers.
>
> * add two log4j2 xml files for GEMFIRE_VERBOSE: log4j2-gemfire_verbose-accept.xml, log4j2-gemfire_verbose-deny.xml
> * add two log4j2 xml files for GEODE_VERBOSE: log4j2-geode_verbose-accept.xml, log4j2-geode_verbose-deny.xml
> * rename test GeodeVerboseLogMarkerIntegrationTest
> * add test methods to verify every combination of behavior for accept and deny xml files
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/Configuration.java PRE-CREATION
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/GeodeVerboseLogMarkerIntegrationTest.java PRE-CREATION
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/LogMarkerJUnitTest.java ec9d257892b64bb6ad2fc58755a448c70ddf538d
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-accept.xml PRE-CREATION
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-deny.xml PRE-CREATION
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-accept.xml PRE-CREATION
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-deny.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52525/diff/
>
>
> Testing
> -------
>
> GeodeVerboseLogMarkerIntegrationTest
>
>
> Thanks,
>
> Kirk Lund
>
>