Re: Review Request 62442: HIVE-17569: Compare filtered output files in BeeLine tests

[Peter Vary]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62442/#review186541
---


Ship it!




Ship It!

- Peter Vary


On Sept. 27, 2017, 3:11 p.m., Marta Kuczora wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62442/
> ---
> 
> (Updated Sept. 27, 2017, 3:11 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Bugs: HIVE-17569
> https://issues.apache.org/jira/browse/HIVE-17569
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Introduce a new property "test.beeline.compare.portable" with the default 
> value false and if this property is set to true, the result of the commands 
> "EXPLAIN", "DESCRIBE EXTENDED" and "DESCRIBE FORMATTED" will be filtered out 
> from the out files before comparing them in BeeLine tests.
> 
> 
> Diffs
> -
> 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
>  9dfc253 
>   itests/util/src/main/java/org/apache/hive/beeline/QFile.java e70ac38 
> 
> 
> Diff: https://reviews.apache.org/r/62442/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>

Re: Review Request 62442: HIVE-17569: Compare filtered output files in BeeLine tests

[Marta Kuczora]

> On Sept. 22, 2017, 3:50 p.m., Peter Vary wrote:
> > Overall looks good. Just a few nits, which might be a matter of taste 
> > anyway. Feel free to object, it you find it unreasonable.
> > 
> > Thanks for the patch!

Thanks a lot Peter for the review. I fixed the issues you raised.


> On Sept. 22, 2017, 3:50 p.m., Peter Vary wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
> > Lines 91 (patched)
> > 
> >
> > I think it would be nice to have the default value as a boolean instead 
> > of a string, and we might want to call this method getBooleanPropertyValue. 
> > What do you think?

Yeah, we can do it like that.


> On Sept. 22, 2017, 3:50 p.m., Peter Vary wrote:
> > itests/util/src/main/java/org/apache/hive/beeline/QFile.java
> > Lines 70 (patched)
> > 
> >
> > We might want to use regexps here where we have separators like 
> > DESCRIBE[\s\n]+EXTENDED - just an example, which probably should be changed 
> > to be valid :)

It makes sense, thanks for pointing it out. I fixed the entries.


- Marta


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62442/#review185995
---


On Sept. 20, 2017, 3:11 p.m., Marta Kuczora wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62442/
> ---
> 
> (Updated Sept. 20, 2017, 3:11 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Bugs: HIVE-17569
> https://issues.apache.org/jira/browse/HIVE-17569
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Introduce a new property "test.beeline.compare.portable" with the default 
> value false and if this property is set to true, the result of the commands 
> "EXPLAIN", "DESCRIBE EXTENDED" and "DESCRIBE FORMATTED" will be filtered out 
> from the out files before comparing them in BeeLine tests.
> 
> 
> Diffs
> -
> 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
>  9dfc253 
>   itests/util/src/main/java/org/apache/hive/beeline/QFile.java e70ac38 
> 
> 
> Diff: https://reviews.apache.org/r/62442/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>

Re: Review Request 62442: HIVE-17569: Compare filtered output files in BeeLine tests

[Marta Kuczora]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62442/
---

(Updated Sept. 27, 2017, 3:11 p.m.)


Review request for hive and Peter Vary.


Changes
---

Fixed the issues found during the review.


Bugs: HIVE-17569
https://issues.apache.org/jira/browse/HIVE-17569


Repository: hive-git


Description
---

Introduce a new property "test.beeline.compare.portable" with the default value 
false and if this property is set to true, the result of the commands 
"EXPLAIN", "DESCRIBE EXTENDED" and "DESCRIBE FORMATTED" will be filtered out 
from the out files before comparing them in BeeLine tests.


Diffs (updated)
-

  
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
 9dfc253 
  itests/util/src/main/java/org/apache/hive/beeline/QFile.java e70ac38 


Diff: https://reviews.apache.org/r/62442/diff/2/

Changes: https://reviews.apache.org/r/62442/diff/1-2/


Testing
---


Thanks,

Marta Kuczora

Re: Review Request 62442: HIVE-17569: Compare filtered output files in BeeLine tests

[Peter Vary]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62442/#review185995
---



Overall looks good. Just a few nits, which might be a matter of taste anyway. 
Feel free to object, it you find it unreasonable.

Thanks for the patch!


itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
Lines 91 (patched)


I think it would be nice to have the default value as a boolean instead of 
a string, and we might want to call this method getBooleanPropertyValue. What 
do you think?



itests/util/src/main/java/org/apache/hive/beeline/QFile.java
Lines 70 (patched)


We might want to use regexps here where we have separators like 
DESCRIBE[\s\n]+EXTENDED - just an example, which probably should be changed to 
be valid :)



itests/util/src/main/java/org/apache/hive/beeline/QFile.java
Lines 328 (patched)


nit: Could we use a proper javadoc comment? Thanks!



itests/util/src/main/java/org/apache/hive/beeline/QFile.java
Lines 416 (patched)


nit: Javadoc comment please :)


- Peter Vary


On Sept. 20, 2017, 3:11 p.m., Marta Kuczora wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62442/
> ---
> 
> (Updated Sept. 20, 2017, 3:11 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Bugs: HIVE-17569
> https://issues.apache.org/jira/browse/HIVE-17569
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Introduce a new property "test.beeline.compare.portable" with the default 
> value false and if this property is set to true, the result of the commands 
> "EXPLAIN", "DESCRIBE EXTENDED" and "DESCRIBE FORMATTED" will be filtered out 
> from the out files before comparing them in BeeLine tests.
> 
> 
> Diffs
> -
> 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
>  9dfc253 
>   itests/util/src/main/java/org/apache/hive/beeline/QFile.java e70ac38 
> 
> 
> Diff: https://reviews.apache.org/r/62442/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>

Review Request 62442: HIVE-17569: Compare filtered output files in BeeLine tests

[Marta Kuczora]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62442/
---

Review request for hive and Peter Vary.


Bugs: HIVE-17569
https://issues.apache.org/jira/browse/HIVE-17569


Repository: hive-git


Description
---

Introduce a new property "test.beeline.compare.portable" with the default value 
false and if this property is set to true, the result of the commands 
"EXPLAIN", "DESCRIBE EXTENDED" and "DESCRIBE FORMATTED" will be filtered out 
from the out files before comparing them in BeeLine tests.


Diffs
-

  
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
 9dfc253 
  itests/util/src/main/java/org/apache/hive/beeline/QFile.java e70ac38 


Diff: https://reviews.apache.org/r/62442/diff/1/


Testing
---


Thanks,

Marta Kuczora