You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Mengwei Ding <me...@gmail.com> on 2013/07/09 22:22:17 UTC
Review Request 12370: SQOOP-996: Sqoop2: create upgrade tests
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12370/
-----------------------------------------------------------
Review request for Sqoop and Jarek Cecho.
Bugs: SQOOP-996
https://issues.apache.org/jira/browse/SQOOP-996
Repository: sqoop-sqoop2
Description
-------
commit b3602f1d6190926eeffa2fa10e81efdd6611092b
Author: Mengwei Ding <me...@cloudera.com>
Date: Wed Jul 3 15:56:49 2013 -0700
SQOOP-996: Sqoop2: create upgrade tests
:100644 100644 0732b2c... 2b6e436... M core/pom.xml
:000000 100644 0000000... 47eb28c... A core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
Diffs
-----
core/pom.xml 0732b2c
core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java PRE-CREATION
Diff: https://reviews.apache.org/r/12370/diff/
Testing
-------
Thanks,
Mengwei Ding
Re: Review Request 12370: SQOOP-996: Sqoop2: create upgrade tests
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12370/#review23597
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On July 18, 2013, 6:44 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12370/
> -----------------------------------------------------------
>
> (Updated July 18, 2013, 6:44 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-996
> https://issues.apache.org/jira/browse/SQOOP-996
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit cef19e8b3f6755f0c25e5b6cb2051a05aba6eef3
> Author: Mengwei Ding <me...@cloudera.com>
> Date: Wed Jul 3 15:56:49 2013 -0700
>
> SQOOP-996: Sqoop2: create upgrade tests
>
> :100644 100644 0732b2c... 2b6e436... M core/pom.xml
> :000000 100644 0000000... 247e165... A core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
>
>
> Diffs
> -----
>
> core/pom.xml 0732b2c
> core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12370/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12370: SQOOP-996: Sqoop2: create upgrade tests
Posted by Mengwei Ding <me...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12370/
-----------------------------------------------------------
(Updated July 18, 2013, 6:44 p.m.)
Review request for Sqoop and Jarek Cecho.
Bugs: SQOOP-996
https://issues.apache.org/jira/browse/SQOOP-996
Repository: sqoop-sqoop2
Description (updated)
-------
commit cef19e8b3f6755f0c25e5b6cb2051a05aba6eef3
Author: Mengwei Ding <me...@cloudera.com>
Date: Wed Jul 3 15:56:49 2013 -0700
SQOOP-996: Sqoop2: create upgrade tests
:100644 100644 0732b2c... 2b6e436... M core/pom.xml
:000000 100644 0000000... 247e165... A core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
Diffs (updated)
-----
core/pom.xml 0732b2c
core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java PRE-CREATION
Diff: https://reviews.apache.org/r/12370/diff/
Testing
-------
Thanks,
Mengwei Ding
Re: Review Request 12370: SQOOP-996: Sqoop2: create upgrade tests
Posted by Jarek Cecho <ja...@apache.org>.
> On July 17, 2013, 9:01 p.m., Jarek Cecho wrote:
> > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java, lines 99-101
> > <https://reviews.apache.org/r/12370/diff/1/?file=319347#file319347line99>
> >
> > If the purpose is to validate if the method is being called, wouldn't i be simpler to mock it with "noop" and use verify() to ensure that it was called?
>
> Mengwei Ding wrote:
> Well, the upgradeConnector() and upgradeFramework() methods are final, which cannot be mocked. So to verify they are being called, I make the first function call inside them throw an exception.
I see that make sense, thank you for the explanation Mengwei!
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12370/#review23316
-----------------------------------------------------------
On July 18, 2013, 6:44 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12370/
> -----------------------------------------------------------
>
> (Updated July 18, 2013, 6:44 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-996
> https://issues.apache.org/jira/browse/SQOOP-996
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit cef19e8b3f6755f0c25e5b6cb2051a05aba6eef3
> Author: Mengwei Ding <me...@cloudera.com>
> Date: Wed Jul 3 15:56:49 2013 -0700
>
> SQOOP-996: Sqoop2: create upgrade tests
>
> :100644 100644 0732b2c... 2b6e436... M core/pom.xml
> :000000 100644 0000000... 247e165... A core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
>
>
> Diffs
> -----
>
> core/pom.xml 0732b2c
> core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12370/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12370: SQOOP-996: Sqoop2: create upgrade tests
Posted by Mengwei Ding <me...@gmail.com>.
> On July 17, 2013, 9:01 p.m., Jarek Cecho wrote:
> > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java, lines 99-101
> > <https://reviews.apache.org/r/12370/diff/1/?file=319347#file319347line99>
> >
> > If the purpose is to validate if the method is being called, wouldn't i be simpler to mock it with "noop" and use verify() to ensure that it was called?
Well, the upgradeConnector() and upgradeFramework() methods are final, which cannot be mocked. So to verify they are being called, I make the first function call inside them throw an exception.
- Mengwei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12370/#review23316
-----------------------------------------------------------
On July 9, 2013, 8:22 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12370/
> -----------------------------------------------------------
>
> (Updated July 9, 2013, 8:22 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-996
> https://issues.apache.org/jira/browse/SQOOP-996
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit b3602f1d6190926eeffa2fa10e81efdd6611092b
> Author: Mengwei Ding <me...@cloudera.com>
> Date: Wed Jul 3 15:56:49 2013 -0700
>
> SQOOP-996: Sqoop2: create upgrade tests
>
> :100644 100644 0732b2c... 2b6e436... M core/pom.xml
> :000000 100644 0000000... 47eb28c... A core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
>
>
> Diffs
> -----
>
> core/pom.xml 0732b2c
> core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12370/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12370: SQOOP-996: Sqoop2: create upgrade tests
Posted by Mengwei Ding <me...@gmail.com>.
> On July 17, 2013, 9:01 p.m., Jarek Cecho wrote:
> > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java, line 189
> > <https://reviews.apache.org/r/12370/diff/1/?file=319347#file319347line189>
> >
> > Nit: "procedure connector upgrade procedure"
Thank you for pointing this out.
- Mengwei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12370/#review23316
-----------------------------------------------------------
On July 9, 2013, 8:22 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12370/
> -----------------------------------------------------------
>
> (Updated July 9, 2013, 8:22 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-996
> https://issues.apache.org/jira/browse/SQOOP-996
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit b3602f1d6190926eeffa2fa10e81efdd6611092b
> Author: Mengwei Ding <me...@cloudera.com>
> Date: Wed Jul 3 15:56:49 2013 -0700
>
> SQOOP-996: Sqoop2: create upgrade tests
>
> :100644 100644 0732b2c... 2b6e436... M core/pom.xml
> :000000 100644 0000000... 47eb28c... A core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
>
>
> Diffs
> -----
>
> core/pom.xml 0732b2c
> core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12370/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12370: SQOOP-996: Sqoop2: create upgrade tests
Posted by Mengwei Ding <me...@gmail.com>.
> On July 17, 2013, 9:01 p.m., Jarek Cecho wrote:
> > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java, lines 149-151
> > <https://reviews.apache.org/r/12370/diff/1/?file=319347#file319347line149>
> >
> > If the purpose is to validate if the method is being called, wouldn't i be simpler to mock it with "noop" and use verify() to ensure that it was called?
For the same reason above.
- Mengwei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12370/#review23316
-----------------------------------------------------------
On July 9, 2013, 8:22 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12370/
> -----------------------------------------------------------
>
> (Updated July 9, 2013, 8:22 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-996
> https://issues.apache.org/jira/browse/SQOOP-996
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit b3602f1d6190926eeffa2fa10e81efdd6611092b
> Author: Mengwei Ding <me...@cloudera.com>
> Date: Wed Jul 3 15:56:49 2013 -0700
>
> SQOOP-996: Sqoop2: create upgrade tests
>
> :100644 100644 0732b2c... 2b6e436... M core/pom.xml
> :000000 100644 0000000... 47eb28c... A core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
>
>
> Diffs
> -----
>
> core/pom.xml 0732b2c
> core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12370/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12370: SQOOP-996: Sqoop2: create upgrade tests
Posted by Mengwei Ding <me...@gmail.com>.
> On July 17, 2013, 9:01 p.m., Jarek Cecho wrote:
> > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java, lines 998-1000
> > <https://reviews.apache.org/r/12370/diff/1/?file=319347#file319347line998>
> >
> > Nit: Can be simplified to:
> > Collections.addAll(connectors, cs);
Thank you, Jarcec.
> On July 17, 2013, 9:01 p.m., Jarek Cecho wrote:
> > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java, lines 1006-1008
> > <https://reviews.apache.org/r/12370/diff/1/?file=319347#file319347line1006>
> >
> > Nit: Can be simplified to:
> >
> > Collections.addAll(jobs, js);
Thank you, Jarcec.
- Mengwei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12370/#review23316
-----------------------------------------------------------
On July 9, 2013, 8:22 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12370/
> -----------------------------------------------------------
>
> (Updated July 9, 2013, 8:22 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-996
> https://issues.apache.org/jira/browse/SQOOP-996
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit b3602f1d6190926eeffa2fa10e81efdd6611092b
> Author: Mengwei Ding <me...@cloudera.com>
> Date: Wed Jul 3 15:56:49 2013 -0700
>
> SQOOP-996: Sqoop2: create upgrade tests
>
> :100644 100644 0732b2c... 2b6e436... M core/pom.xml
> :000000 100644 0000000... 47eb28c... A core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
>
>
> Diffs
> -----
>
> core/pom.xml 0732b2c
> core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12370/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12370: SQOOP-996: Sqoop2: create upgrade tests
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12370/#review23316
-----------------------------------------------------------
Thank you Mengwei for working on this JIRA! Couple of notes:
core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
<https://reviews.apache.org/r/12370/#comment47196>
If the purpose is to validate if the method is being called, wouldn't i be simpler to mock it with "noop" and use verify() to ensure that it was called?
core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
<https://reviews.apache.org/r/12370/#comment47210>
If the purpose is to validate if the method is being called, wouldn't i be simpler to mock it with "noop" and use verify() to ensure that it was called?
core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
<https://reviews.apache.org/r/12370/#comment47211>
Nit: "procedure connector upgrade procedure"
core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
<https://reviews.apache.org/r/12370/#comment47212>
Nit: Can be simplified to:
Collections.addAll(connectors, cs);
core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
<https://reviews.apache.org/r/12370/#comment47213>
Nit: Can be simplified to:
Collections.addAll(jobs, js);
Jarcec
- Jarek Cecho
On July 9, 2013, 8:22 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12370/
> -----------------------------------------------------------
>
> (Updated July 9, 2013, 8:22 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-996
> https://issues.apache.org/jira/browse/SQOOP-996
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit b3602f1d6190926eeffa2fa10e81efdd6611092b
> Author: Mengwei Ding <me...@cloudera.com>
> Date: Wed Jul 3 15:56:49 2013 -0700
>
> SQOOP-996: Sqoop2: create upgrade tests
>
> :100644 100644 0732b2c... 2b6e436... M core/pom.xml
> :000000 100644 0000000... 47eb28c... A core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java
>
>
> Diffs
> -----
>
> core/pom.xml 0732b2c
> core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12370/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mengwei Ding
>
>