sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Abraham Fine <...@abrahamfine.com>
Subject Re: Review Request 42343: SQOOP-2790: Remove the duplicate test cases in shell module
Date Tue, 26 Jan 2016 18:33:26 GMT


> On Jan. 25, 2016, 6:33 p.m., Abraham Fine wrote:
> > why are we doing this? don't the integration tests and unit tests serve two different
purposes?
> > 
> > the unit tests test that the shell is sending the right message to the server and
the integration tests ensure that the server is performing the correct behavior based on the
message that the shell sends
> 
> Colin Ma wrote:
>     Currently, the unit tests don't test sending the right message, actutally, it tests
the server's behavior by Mock. The purpose of creating integration tests is to replace these
test cases.
>     This change doesn't reduce the test scope, just delete the meaningless and duplicated
test cases.
>     For the code manitenance, also move some negative test cases to integration test.

is that always the case?

for instance, what about the set command. isn't testing that better suited for unit tests
than integration?

in other words, aren't shell tests better suited to be run on the unit test level with a mock
"client" rather than a mock server?


- Abraham


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


On Jan. 15, 2016, 7:31 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42343/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2016, 7:31 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> The test cases for shell will be refactored as following: 
> 1. Because the integration tests are created for shell, remove the duplicated cases in
shell module. 
> 2. Move the negative test cases from shell module to integration test.
> 
> The authorization test cases won't be changed in this ticket, and keep them in shell
module for now.
> 
> 
> Diffs
> -----
> 
>   shell/src/test/java/org/apache/sqoop/shell/TestCloneCommand.java 4515b1c 
>   shell/src/test/java/org/apache/sqoop/shell/TestCreateCommand.java c1c23db 
>   shell/src/test/java/org/apache/sqoop/shell/TestDeleteCommand.java 47a8f87 
>   shell/src/test/java/org/apache/sqoop/shell/TestDisableCommand.java 9e4e532 
>   shell/src/test/java/org/apache/sqoop/shell/TestEnableCommand.java 4c52448 
>   shell/src/test/java/org/apache/sqoop/shell/TestSetCommand.java 93e1e3e 
>   shell/src/test/java/org/apache/sqoop/shell/TestShowCommand.java 49affa3 
>   shell/src/test/java/org/apache/sqoop/shell/TestStartCommand.java 7dc407c 
>   shell/src/test/java/org/apache/sqoop/shell/TestStatusCommand.java ce01842 
>   shell/src/test/java/org/apache/sqoop/shell/TestStopCommand.java ca9d03b 
>   shell/src/test/java/org/apache/sqoop/shell/TestUpdateCommand.java 469ded7 
>   test/src/test/java/org/apache/sqoop/integration/shell/CloneCommandTest.java bcf6334

>   test/src/test/java/org/apache/sqoop/integration/shell/CreateCommandTest.java 81b7c8a

>   test/src/test/java/org/apache/sqoop/integration/shell/DeleteCommandTest.java 8ed8570

>   test/src/test/java/org/apache/sqoop/integration/shell/DisableCommandTest.java e524312

>   test/src/test/java/org/apache/sqoop/integration/shell/EnableCommandTest.java b104348

>   test/src/test/java/org/apache/sqoop/integration/shell/SetCommandTest.java f673a58 
>   test/src/test/java/org/apache/sqoop/integration/shell/StartCommandTest.java 5504586

>   test/src/test/java/org/apache/sqoop/integration/shell/StatusCommandTest.java 7294b05

>   test/src/test/java/org/apache/sqoop/integration/shell/StopCommandTest.java 69110b2

>   test/src/test/java/org/apache/sqoop/integration/shell/UpdateCommandTest.java f9a35fe

> 
> Diff: https://reviews.apache.org/r/42343/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message