You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Kinga Marton via Review Board <no...@reviews.apache.org> on 2019/05/30 09:35:48 UTC
Review Request 70762: OOZIE-3499 [Java 11] Fix
TestLiteWorkflowAppParser
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70762/
-----------------------------------------------------------
Review request for oozie and Andras Salamon.
Repository: oozie-git
Description
-------
All the tests from TestLiteWorkflowAppParser, where global configurations are used are failing because with Java11 the order of the properties is not the same as with Java8.
Diffs
-----
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java 61e386c43
Diff: https://reviews.apache.org/r/70762/diff/1/
Testing
-------
Thanks,
Kinga Marton
Re: Review Request 70762: OOZIE-3499 [Java 11] Fix
TestLiteWorkflowAppParser
Posted by Andras Salamon <an...@melda.info>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70762/#review215601
-----------------------------------------------------------
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 72-73 (original), 75-76 (patched)
<https://reviews.apache.org/r/70762/#comment302340>
Can we extract it to a method?
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Line 77 (original), 80 (patched)
<https://reviews.apache.org/r/70762/#comment302334>
Please use the diamond operator, to avoid raw types.
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Line 86 (original), 89 (patched)
<https://reviews.apache.org/r/70762/#comment302338>
This extractConfigurationFromXML method is quote useful, but we always call it like this:
extractConfigurationFromXML(cleanupXml(d))
Could we embed the cleanupXml function into this method? Or maybe to a new cleanupAndExtractConfigurationfromXML?
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 98-99 (patched)
<https://reviews.apache.org/r/70762/#comment302335>
Please add assert messages.
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 102 (patched)
<https://reviews.apache.org/r/70762/#comment302339>
I'd rename it to expectedConfig (or something similar) to keep it constistent with the other expect prefixed variable names.
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 105 (patched)
<https://reviews.apache.org/r/70762/#comment302336>
Please add assert message.
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 109-110 (original), 112-113 (patched)
<https://reviews.apache.org/r/70762/#comment302341>
Can we extract it to a method?
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 120-122 (original), 123-125 (patched)
<https://reviews.apache.org/r/70762/#comment302337>
Please add assert message.
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 149-150 (original), 132-133 (patched)
<https://reviews.apache.org/r/70762/#comment302342>
Can we extract it to a method?
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 184-185 (original), 149-150 (patched)
<https://reviews.apache.org/r/70762/#comment302343>
Can we extract it to a method?
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 221-222 (original), 167-168 (patched)
<https://reviews.apache.org/r/70762/#comment302344>
Can we extract it to a method?
- Andras Salamon
On May 30, 2019, 9:35 a.m., Kinga Marton wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70762/
> -----------------------------------------------------------
>
> (Updated May 30, 2019, 9:35 a.m.)
>
>
> Review request for oozie and Andras Salamon.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> All the tests from TestLiteWorkflowAppParser, where global configurations are used are failing because with Java11 the order of the properties is not the same as with Java8.
>
>
> Diffs
> -----
>
> core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java 61e386c43
>
>
> Diff: https://reviews.apache.org/r/70762/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kinga Marton
>
>
Re: Review Request 70762: OOZIE-3499 [Java 11] Fix
TestLiteWorkflowAppParser
Posted by Andras Salamon <an...@melda.info>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70762/#review215614
-----------------------------------------------------------
Ship it!
Ship It!
- Andras Salamon
On May 31, 2019, 12:30 p.m., Kinga Marton wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70762/
> -----------------------------------------------------------
>
> (Updated May 31, 2019, 12:30 p.m.)
>
>
> Review request for oozie and Andras Salamon.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> All the tests from TestLiteWorkflowAppParser, where global configurations are used are failing because with Java11 the order of the properties is not the same as with Java8.
>
>
> Diffs
> -----
>
> core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java 61e386c43
>
>
> Diff: https://reviews.apache.org/r/70762/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kinga Marton
>
>
Re: Review Request 70762: OOZIE-3499 [Java 11] Fix
TestLiteWorkflowAppParser
Posted by Kinga Marton via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70762/
-----------------------------------------------------------
(Updated May 31, 2019, 12:30 p.m.)
Review request for oozie and Andras Salamon.
Repository: oozie-git
Description
-------
All the tests from TestLiteWorkflowAppParser, where global configurations are used are failing because with Java11 the order of the properties is not the same as with Java8.
Diffs (updated)
-----
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java 61e386c43
Diff: https://reviews.apache.org/r/70762/diff/3/
Changes: https://reviews.apache.org/r/70762/diff/2-3/
Testing
-------
Thanks,
Kinga Marton
Re: Review Request 70762: OOZIE-3499 [Java 11] Fix
TestLiteWorkflowAppParser
Posted by Andras Salamon <an...@melda.info>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70762/#review215613
-----------------------------------------------------------
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Line 74 (original), 77 (patched)
<https://reviews.apache.org/r/70762/#comment302362>
Please do not use raw types. This would be better:
Map<String,String> expectedConfigs = new HashMap<>();
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 105 (patched)
<https://reviews.apache.org/r/70762/#comment302367>
Couldn't we use Map.Entry<String, String> and avoid the toString() methods?
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 118 (patched)
<https://reviews.apache.org/r/70762/#comment302363>
Please do not use raw types.
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Line 192 (original), 135 (patched)
<https://reviews.apache.org/r/70762/#comment302364>
Please do not use raw types.
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Lines 149 (patched)
<https://reviews.apache.org/r/70762/#comment302365>
Please do not use raw types.
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
Line 229 (original), 164 (patched)
<https://reviews.apache.org/r/70762/#comment302366>
Please do not use raw types.
- Andras Salamon
On May 31, 2019, 11:28 a.m., Kinga Marton wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70762/
> -----------------------------------------------------------
>
> (Updated May 31, 2019, 11:28 a.m.)
>
>
> Review request for oozie and Andras Salamon.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> All the tests from TestLiteWorkflowAppParser, where global configurations are used are failing because with Java11 the order of the properties is not the same as with Java8.
>
>
> Diffs
> -----
>
> core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java 61e386c43
>
>
> Diff: https://reviews.apache.org/r/70762/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kinga Marton
>
>
Re: Review Request 70762: OOZIE-3499 [Java 11] Fix
TestLiteWorkflowAppParser
Posted by Kinga Marton via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70762/
-----------------------------------------------------------
(Updated May 31, 2019, 11:28 a.m.)
Review request for oozie and Andras Salamon.
Repository: oozie-git
Description
-------
All the tests from TestLiteWorkflowAppParser, where global configurations are used are failing because with Java11 the order of the properties is not the same as with Java8.
Diffs (updated)
-----
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java 61e386c43
Diff: https://reviews.apache.org/r/70762/diff/2/
Changes: https://reviews.apache.org/r/70762/diff/1-2/
Testing
-------
Thanks,
Kinga Marton