You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@beam.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2019/12/20 21:02:00 UTC
[jira] [Work logged] (BEAM-9000) Java Test Assertions without
toString for GenericJson subclasses
[ https://issues.apache.org/jira/browse/BEAM-9000?focusedWorklogId=361858&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-361858 ]
ASF GitHub Bot logged work on BEAM-9000:
----------------------------------------
Author: ASF GitHub Bot
Created on: 20/Dec/19 21:01
Start Date: 20/Dec/19 21:01
Worklog Time Spent: 10m
Work Description: suztomo commented on pull request #10441: [BEAM-9000] Java Test Assertions without toString for GenericJson subclasses
URL: https://github.com/apache/beam/pull/10441
Option 3 "Option3: Check JSON equality via JSONassert" in
https://issues.apache.org/jira/projects/BEAM/issues/BEAM-9000
As of now, there are many tests that assert on `toString()` of GenericJson subclasses.
```
CounterUpdate result = testObject.transform(monitoringInfo);
assertEquals(
"{cumulative=true, integer={highBits=0, lowBits=0}, "
+ "nameAndKind={kind=SUM, "
+ "name=transformedValue-ElementCount}}",
result.toString());
```
This style is prone to unnecessary maintenance of the test code when upgrading dependencies. Dependencies may change the internal ordering of fields and trivial change in {{toString()}}. In fact, BEAM-8695 is suspended partly because of these tests.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 361858)
Remaining Estimate: 0h
Time Spent: 10m
> Java Test Assertions without toString for GenericJson subclasses
> ----------------------------------------------------------------
>
> Key: BEAM-9000
> URL: https://issues.apache.org/jira/browse/BEAM-9000
> Project: Beam
> Issue Type: Improvement
> Components: testing
> Reporter: Tomo Suzuki
> Assignee: Tomo Suzuki
> Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> As of now, there are many tests that assert on {{toString()}} of objects.
> {code:java}
> CounterUpdate result = testObject.transform(monitoringInfo);
> assertEquals(
> "{cumulative=true, integer={highBits=0, lowBits=0}, "
> + "nameAndKind={kind=SUM, "
> + "name=transformedValue-ElementCount}}",
> result.toString());
> {code}
> This style is prone to unnecessary maintenance of the test code when upgrading dependencies. Dependencies may change the internal ordering of fields and trivial change in {{toString()}}. In BEAM-8695, where I tried to upgrade google-http-client, there are ~30 comparison failure due to this {{toString}} assertions.
> They are subclasses of {{com.google.api.client.json.GenericJson}}.
> Several options to enhance these assertions.
> h1. Option 1: Assertion using Map
> Leveraging the fact that GenericJson is a subclass of AbstractMap<String, Object>, the assertion can be written as
> {code:java}
> ImmutableMap<String, Object> expected = ImmutableMap.of("cumulative", true,
> "integer", ImmutableMap.of("highBits", 0, "lowBits", 0),
> "nameAndKind", ImmutableMap.of("kind", "SUM", "name", "transformedValue-ElementCount"));
> assertEquals(expected, (Map<String, Object>)result);
> {code}
> Credit: Ben Whitehead.
> h1. Option 2: Create assertEqualsOnJson
> Leveraging the fact that instance of GenericJson can be instantiated through JSON, the assertion can be written as
> {code:java}
> assertEqualsOnJson(
> "{\"cumulative\":true, \"integer\":{\"highBits\":0, \"lowBits\":0}, "
> + "\"nameAndKind\":{\"kind\":\"SUM\", "
> + "\"name\":\"transformedValue-ElementCount\"}}",
> result);
> {code}
>
> {{assertEqualsOnJson}} is implemented as below. The following field and methods should go to shared test utility class (sdks/testing?)
> {code:java}
> private static final JacksonFactory jacksonFactory = JacksonFactory.getDefaultInstance();
> public static <T extends GenericJson> void assertEqualsOnJson(String expectedJsonText, T actual) {
> CounterUpdate expected = parse(expectedJsonText, CounterUpdate.class);
> assertEquals(expected, actual);
> }
> public static <T extends GenericJson> T parse(String text, Class<T> clazz) {
> try {
> JsonParser parser = jacksonFactory.createJsonParser(text);
> return parser.parse(clazz);
> } catch (IOException ex) {
> throw new IllegalArgumentException("Could not parse the text as " + clazz, ex);
> }
> }
> {code}
> A feature request to handle escaping double quotes via JacksonFactory: [https://github.com/googleapis/google-http-java-client/issues/923]
>
> h1. Option3: Check JSON equality via JSONassert
> * https://github.com/skyscreamer/JSONassert
> * https://github.com/hertzsprung/hamcrest-json (Not using as last commit was in 2012)
> The JSONassert example does not carry quoted double quote characters. The implementation would be converting actual object into JSON object and calling {{JSONAssert.assertEqual}}.
> Credit: Luke Cwik
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)