You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Boglarka Egyed <eg...@gmail.com> on 2016/12/22 23:25:28 UTC
Review Request 55001: Clean up expected exception logic in tests -
part I.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55001/
-----------------------------------------------------------
Review request for Sqoop, Attila Szabo and Anna Szonyi.
Bugs: SQOOP-3091
https://issues.apache.org/jira/browse/SQOOP-3091
Repository: sqoop-trunk
Description
-------
Normalizing test cases where we except an exception using JUnit's ExpectedException rule to make the code mor clean and self-explanatory.
Diffs
-----
src/test/com/cloudera/sqoop/TestAvroExport.java b51313895e8e5b5aa45aa7169b7200d50f53a792
src/test/com/cloudera/sqoop/TestConnFactory.java 59c3455401c70acbac8a4d140639e611c03d30a5
src/test/com/cloudera/sqoop/TestExportUpdate.java 95d7b6ae80c64ac4b6a574ada1d02e138d7c963a
src/test/com/cloudera/sqoop/TestParquetExport.java c6ddef6a82e99c8b6c03e0d71584bed50b60355b
src/test/com/cloudera/sqoop/TestSqoopOptions.java d95f904347191f84caa106a30a513978114eb5f0
src/test/com/cloudera/sqoop/TestTargetDir.java 7aad7e15dcde092e0c674d5d818be9750d7347a6
src/test/org/apache/sqoop/TestExportUsingProcedure.java cf5e2cd07ba3f322e53ebe9c3852eae13f78d133
Diff: https://reviews.apache.org/r/55001/diff/
Testing
-------
ant test, ant clean test
Thanks,
Boglarka Egyed
Re: Review Request 55001: Clean up expected exception logic in tests
- part I.
Posted by Erzsebet Szilagyi <er...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55001/#review160330
-----------------------------------------------------------
Ship it!
Thank you for improving our testing!
- Erzsebet Szilagyi
On Dec. 28, 2016, 4:57 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55001/
> -----------------------------------------------------------
>
> (Updated Dec. 28, 2016, 4:57 p.m.)
>
>
> Review request for Sqoop, Attila Szabo and Anna Szonyi.
>
>
> Bugs: SQOOP-3091
> https://issues.apache.org/jira/browse/SQOOP-3091
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Normalizing test cases where we except an exception using JUnit's ExpectedException rule to make the code mor clean and self-explanatory.
>
>
> Diffs
> -----
>
> src/test/com/cloudera/sqoop/TestAvroExport.java b51313895e8e5b5aa45aa7169b7200d50f53a792
> src/test/com/cloudera/sqoop/TestConnFactory.java 59c3455401c70acbac8a4d140639e611c03d30a5
> src/test/com/cloudera/sqoop/TestExportUpdate.java 95d7b6ae80c64ac4b6a574ada1d02e138d7c963a
> src/test/com/cloudera/sqoop/TestParquetExport.java c6ddef6a82e99c8b6c03e0d71584bed50b60355b
> src/test/com/cloudera/sqoop/TestSqoopOptions.java d95f904347191f84caa106a30a513978114eb5f0
> src/test/com/cloudera/sqoop/TestTargetDir.java 7aad7e15dcde092e0c674d5d818be9750d7347a6
> src/test/org/apache/sqoop/TestExportUsingProcedure.java cf5e2cd07ba3f322e53ebe9c3852eae13f78d133
>
> Diff: https://reviews.apache.org/r/55001/diff/
>
>
> Testing
> -------
>
> ant test, ant clean test
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 55001: Clean up expected exception logic in tests
- part I.
Posted by Boglarka Egyed <eg...@gmail.com>.
> On Jan. 3, 2017, 11:05 a.m., Anna Szonyi wrote:
> > Thanks Bogi!
> >
> > This is a great improvement for the tests, it's been quite annyoing :)
> > I've opened a jira to remove all junit3 related classes, conventions from the code as well.
> >
> > Thanks,
> > /Anna
Thanks Anna for the review and the info! I will remove the static suite methods after your change has been committed.
- Boglarka
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55001/#review160367
-----------------------------------------------------------
On Dec. 28, 2016, 3:57 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55001/
> -----------------------------------------------------------
>
> (Updated Dec. 28, 2016, 3:57 p.m.)
>
>
> Review request for Sqoop, Attila Szabo and Anna Szonyi.
>
>
> Bugs: SQOOP-3091
> https://issues.apache.org/jira/browse/SQOOP-3091
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Normalizing test cases where we except an exception using JUnit's ExpectedException rule to make the code mor clean and self-explanatory.
>
>
> Diffs
> -----
>
> src/test/com/cloudera/sqoop/TestAvroExport.java b51313895e8e5b5aa45aa7169b7200d50f53a792
> src/test/com/cloudera/sqoop/TestConnFactory.java 59c3455401c70acbac8a4d140639e611c03d30a5
> src/test/com/cloudera/sqoop/TestExportUpdate.java 95d7b6ae80c64ac4b6a574ada1d02e138d7c963a
> src/test/com/cloudera/sqoop/TestParquetExport.java c6ddef6a82e99c8b6c03e0d71584bed50b60355b
> src/test/com/cloudera/sqoop/TestSqoopOptions.java d95f904347191f84caa106a30a513978114eb5f0
> src/test/com/cloudera/sqoop/TestTargetDir.java 7aad7e15dcde092e0c674d5d818be9750d7347a6
> src/test/org/apache/sqoop/TestExportUsingProcedure.java cf5e2cd07ba3f322e53ebe9c3852eae13f78d133
>
> Diff: https://reviews.apache.org/r/55001/diff/
>
>
> Testing
> -------
>
> ant test, ant clean test
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 55001: Clean up expected exception logic in tests
- part I.
Posted by Anna Szonyi <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55001/#review160367
-----------------------------------------------------------
Ship it!
Thanks Bogi!
This is a great improvement for the tests, it's been quite annyoing :)
I've opened a jira to remove all junit3 related classes, conventions from the code as well.
Thanks,
/Anna
- Anna Szonyi
On Dec. 28, 2016, 3:57 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55001/
> -----------------------------------------------------------
>
> (Updated Dec. 28, 2016, 3:57 p.m.)
>
>
> Review request for Sqoop, Attila Szabo and Anna Szonyi.
>
>
> Bugs: SQOOP-3091
> https://issues.apache.org/jira/browse/SQOOP-3091
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Normalizing test cases where we except an exception using JUnit's ExpectedException rule to make the code mor clean and self-explanatory.
>
>
> Diffs
> -----
>
> src/test/com/cloudera/sqoop/TestAvroExport.java b51313895e8e5b5aa45aa7169b7200d50f53a792
> src/test/com/cloudera/sqoop/TestConnFactory.java 59c3455401c70acbac8a4d140639e611c03d30a5
> src/test/com/cloudera/sqoop/TestExportUpdate.java 95d7b6ae80c64ac4b6a574ada1d02e138d7c963a
> src/test/com/cloudera/sqoop/TestParquetExport.java c6ddef6a82e99c8b6c03e0d71584bed50b60355b
> src/test/com/cloudera/sqoop/TestSqoopOptions.java d95f904347191f84caa106a30a513978114eb5f0
> src/test/com/cloudera/sqoop/TestTargetDir.java 7aad7e15dcde092e0c674d5d818be9750d7347a6
> src/test/org/apache/sqoop/TestExportUsingProcedure.java cf5e2cd07ba3f322e53ebe9c3852eae13f78d133
>
> Diff: https://reviews.apache.org/r/55001/diff/
>
>
> Testing
> -------
>
> ant test, ant clean test
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 55001: Clean up expected exception logic in tests
- part I.
Posted by Attila Szabo <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55001/#review161204
-----------------------------------------------------------
Ship it!
Ship It!
- Attila Szabo
On Dec. 28, 2016, 3:57 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55001/
> -----------------------------------------------------------
>
> (Updated Dec. 28, 2016, 3:57 p.m.)
>
>
> Review request for Sqoop, Attila Szabo and Anna Szonyi.
>
>
> Bugs: SQOOP-3091
> https://issues.apache.org/jira/browse/SQOOP-3091
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Normalizing test cases where we except an exception using JUnit's ExpectedException rule to make the code mor clean and self-explanatory.
>
>
> Diffs
> -----
>
> src/test/com/cloudera/sqoop/TestAvroExport.java b51313895e8e5b5aa45aa7169b7200d50f53a792
> src/test/com/cloudera/sqoop/TestConnFactory.java 59c3455401c70acbac8a4d140639e611c03d30a5
> src/test/com/cloudera/sqoop/TestExportUpdate.java 95d7b6ae80c64ac4b6a574ada1d02e138d7c963a
> src/test/com/cloudera/sqoop/TestParquetExport.java c6ddef6a82e99c8b6c03e0d71584bed50b60355b
> src/test/com/cloudera/sqoop/TestSqoopOptions.java d95f904347191f84caa106a30a513978114eb5f0
> src/test/com/cloudera/sqoop/TestTargetDir.java 7aad7e15dcde092e0c674d5d818be9750d7347a6
> src/test/org/apache/sqoop/TestExportUsingProcedure.java cf5e2cd07ba3f322e53ebe9c3852eae13f78d133
>
> Diff: https://reviews.apache.org/r/55001/diff/
>
>
> Testing
> -------
>
> ant test, ant clean test
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 55001: Clean up expected exception logic in tests
- part I.
Posted by Boglarka Egyed <eg...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55001/
-----------------------------------------------------------
(Updated Dec. 28, 2016, 3:57 p.m.)
Review request for Sqoop, Attila Szabo and Anna Szonyi.
Changes
-------
Add comment for Junit3 style static suite methods
Bugs: SQOOP-3091
https://issues.apache.org/jira/browse/SQOOP-3091
Repository: sqoop-trunk
Description
-------
Normalizing test cases where we except an exception using JUnit's ExpectedException rule to make the code mor clean and self-explanatory.
Diffs (updated)
-----
src/test/com/cloudera/sqoop/TestAvroExport.java b51313895e8e5b5aa45aa7169b7200d50f53a792
src/test/com/cloudera/sqoop/TestConnFactory.java 59c3455401c70acbac8a4d140639e611c03d30a5
src/test/com/cloudera/sqoop/TestExportUpdate.java 95d7b6ae80c64ac4b6a574ada1d02e138d7c963a
src/test/com/cloudera/sqoop/TestParquetExport.java c6ddef6a82e99c8b6c03e0d71584bed50b60355b
src/test/com/cloudera/sqoop/TestSqoopOptions.java d95f904347191f84caa106a30a513978114eb5f0
src/test/com/cloudera/sqoop/TestTargetDir.java 7aad7e15dcde092e0c674d5d818be9750d7347a6
src/test/org/apache/sqoop/TestExportUsingProcedure.java cf5e2cd07ba3f322e53ebe9c3852eae13f78d133
Diff: https://reviews.apache.org/r/55001/diff/
Testing
-------
ant test, ant clean test
Thanks,
Boglarka Egyed
Re: Review Request 55001: Clean up expected exception logic in tests
- part I.
Posted by Boglarka Egyed <eg...@gmail.com>.
> On Dec. 23, 2016, 12:08 a.m., Attila Szabo wrote:
> > Hey Bogi,
> >
> > The change looks good, I'm concerned about only one thing:
> > why do we have these old Junit3 style static suite methods, if we change to Junit4 running environment, annotations, expectedexceptions, etc.?
> >
> > Thanks for the clarificaiton in advance!
> >
> > Attila
Hi Attila,
Many thanks for your review!
I added these methods as a workaround because ant keeps falling back to JUnit3 during the test executions. I found this solution in TestDirectImport, HCatalogExportTest and HCatalogImportTest classes too. If you have any suggestion for a better solution please let me know.
I have added a comment to the methods for better understanding in the future.
Thanks,
Bogi
- Boglarka
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55001/#review160021
-----------------------------------------------------------
On Dec. 28, 2016, 3:57 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55001/
> -----------------------------------------------------------
>
> (Updated Dec. 28, 2016, 3:57 p.m.)
>
>
> Review request for Sqoop, Attila Szabo and Anna Szonyi.
>
>
> Bugs: SQOOP-3091
> https://issues.apache.org/jira/browse/SQOOP-3091
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Normalizing test cases where we except an exception using JUnit's ExpectedException rule to make the code mor clean and self-explanatory.
>
>
> Diffs
> -----
>
> src/test/com/cloudera/sqoop/TestAvroExport.java b51313895e8e5b5aa45aa7169b7200d50f53a792
> src/test/com/cloudera/sqoop/TestConnFactory.java 59c3455401c70acbac8a4d140639e611c03d30a5
> src/test/com/cloudera/sqoop/TestExportUpdate.java 95d7b6ae80c64ac4b6a574ada1d02e138d7c963a
> src/test/com/cloudera/sqoop/TestParquetExport.java c6ddef6a82e99c8b6c03e0d71584bed50b60355b
> src/test/com/cloudera/sqoop/TestSqoopOptions.java d95f904347191f84caa106a30a513978114eb5f0
> src/test/com/cloudera/sqoop/TestTargetDir.java 7aad7e15dcde092e0c674d5d818be9750d7347a6
> src/test/org/apache/sqoop/TestExportUsingProcedure.java cf5e2cd07ba3f322e53ebe9c3852eae13f78d133
>
> Diff: https://reviews.apache.org/r/55001/diff/
>
>
> Testing
> -------
>
> ant test, ant clean test
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 55001: Clean up expected exception logic in tests
- part I.
Posted by Attila Szabo <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55001/#review160021
-----------------------------------------------------------
Hey Bogi,
The change looks good, I'm concerned about only one thing:
why do we have these old Junit3 style static suite methods, if we change to Junit4 running environment, annotations, expectedexceptions, etc.?
Thanks for the clarificaiton in advance!
Attila
- Attila Szabo
On Dec. 22, 2016, 11:25 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55001/
> -----------------------------------------------------------
>
> (Updated Dec. 22, 2016, 11:25 p.m.)
>
>
> Review request for Sqoop, Attila Szabo and Anna Szonyi.
>
>
> Bugs: SQOOP-3091
> https://issues.apache.org/jira/browse/SQOOP-3091
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Normalizing test cases where we except an exception using JUnit's ExpectedException rule to make the code mor clean and self-explanatory.
>
>
> Diffs
> -----
>
> src/test/com/cloudera/sqoop/TestAvroExport.java b51313895e8e5b5aa45aa7169b7200d50f53a792
> src/test/com/cloudera/sqoop/TestConnFactory.java 59c3455401c70acbac8a4d140639e611c03d30a5
> src/test/com/cloudera/sqoop/TestExportUpdate.java 95d7b6ae80c64ac4b6a574ada1d02e138d7c963a
> src/test/com/cloudera/sqoop/TestParquetExport.java c6ddef6a82e99c8b6c03e0d71584bed50b60355b
> src/test/com/cloudera/sqoop/TestSqoopOptions.java d95f904347191f84caa106a30a513978114eb5f0
> src/test/com/cloudera/sqoop/TestTargetDir.java 7aad7e15dcde092e0c674d5d818be9750d7347a6
> src/test/org/apache/sqoop/TestExportUsingProcedure.java cf5e2cd07ba3f322e53ebe9c3852eae13f78d133
>
> Diff: https://reviews.apache.org/r/55001/diff/
>
>
> Testing
> -------
>
> ant test, ant clean test
>
>
> Thanks,
>
> Boglarka Egyed
>
>