sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request 39817: Sqoop2: Add tests for ConfigFiller
Date Tue, 03 Nov 2015 02:11:35 GMT


> On Nov. 2, 2015, 11:09 p.m., Jarek Cecho wrote:
> > shell/src/main/java/org/apache/sqoop/shell/utils/ConfigFiller.java, lines 65-70
> > <https://reviews.apache.org/r/39817/diff/3/?file=1113822#file1113822line65>
> >
> >     I'm wondering what is the purpose of the test flag? Can we drop it?
> >     
> >     I would much rather to test the class without any special flags to get as close
to "production" as possible.
> 
> Dian Fu wrote:
>     In methods like fillInput...WithBundle, reader.putString(String str) is called. This
method will write the specified string into the internal buffer of ConsoleReader. As we have
already filled the internal buffer of ConsoleReader with our test data, the writen string
by reader.putString() will be mixed with the test data. I agree we should test as close to
"production" as possible. But the functionality of reader.putString(String str) is just to
display some string on the console and so disabling it during test won't make the test much
different from "production".

I think that I've seen the problem you're describing when I played with the patch. What I've
seen is that some of the tests are reusing single Input multiple times and if if the reused
one alredy had a value, the it got "messed up". I do however feel that it's a test problem
because that is an expected behavior. If the input have a value, then user is editing it rather
then inserting it from scratch.

I do feel that the right solution here is to either not reuse the inputs or call setEmpty()
before each test rather then conditionally skip valied pieces of code.


- Jarek


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


On Nov. 3, 2015, 1:54 a.m., Dian Fu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39817/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2015, 1:54 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2648
>     https://issues.apache.org/jira/browse/SQOOP-2648
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> This JIRA add tests for ConfigFiller. ConfigFiller is used by interactive mode.
> 
> 
> Diffs
> -----
> 
>   shell/src/main/java/org/apache/sqoop/shell/utils/ConfigFiller.java 6a2a96d 
>   shell/src/test/java/org/apache/sqoop/shell/utils/TestConfigFiller.java PRE-CREATION

> 
> Diff: https://reviews.apache.org/r/39817/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dian Fu
> 
>


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