> On Aug. 31, 2018, 1:24 p.m., Boglarka Egyed wrote:
> > Hi Nguyen,
> >
> > Many thanks for taking care of this huge effort! Having well categorized
> > tests is a long-awaited improvement Sqoop, it will ease the life of every
> > developer.
> >
> > In general your change looks good to me however I have three findings:
> >
> > 1.) New test classes have been introduced recently in SQOOP-3348,
> > SQOOP-3363 and SQOOP-3368 testing the Sqoop-S3 integration. I think it
> > would make sense to have a new test category for the S3 related tests
> > similarly to the KerberizedTest category.
> >
> > 2.) There should be well defined, thorough definitions of each test
> > categories and these should be well documented somewhere. This is something
> > that could be a result of a discussion amongst the contributors. After
> > creating such definitions they could be added as javadocs to the test
> > category interfaces as well as included in the Sqoop Developer’s Guide
> > (changes would be needed under sqoop/src/docs/dev).
> >
> > 3.) It would be also very useful to have exact commands described to run
> > specific categories in the COMPILING.txt.
> >
> > Thank you very much,
> > Bogi
>
> Nguyen Truong wrote:
> Hi Boglarka,
>
> Thanks for your review. I will go on and add categories for the new tests.
>
> I don't know about the S3 tests. Could you please let me know more why
> they should be under a separate category as KerberizedTest? Thank you very
> much.
>
> I will update the COMPILING.txt after we come up with the final decision
> on the categories.
>
> Thanks,
> Nguyen
>
> Boglarka Egyed wrote:
> S3 tests should be in a separate test category as they test the
> integration with the Amazon S3 cloud service that is a third party side just
> like an RDBMS. These tests also require AWS credentials to access S3 and they
> run only if these credentials are provided via the
> -Ds3.generator.command= property as well as the
> target S3 location via the -Ds3.bucket.url= property.
> The S3 tests that should be in a separate category are under
> sqoop/src/test/org/apache/sqoop/s3/
> The S3 related test case added in SQOOP-3368 is a simple unit test case
> though, it doesn't test the integration with S3 just the sqoop option
> validation mechanism.
>
> I will also think about the definitions of the test categories and will
> share my thoughts later.
Hey Natalie,
I think I ran a bit forward with my previous comments. Let's skip the S3
category now, it can be added in a later patch as well. Please ignore my words
above.
We had a discussion with Szabolcs and Fero and here is our recommendation for
the test categorie definitions:
SqoopTest
-
UnitTest:
A unit test shall test one class at a time having it's dependencies mocked.
A unit test shall not start a mini cluster nor an embedded database and it
shall not use a JDBC driver.
IntegrationTest:
An integration test shall test if independently developed classes work together
correctly.
An integration test checks a whole scenario and thus may start mini clusters or
embedded databases and may connect to external resources like RDBMS instances.
ManualTest:
Deprecated category, shall not be used nor extended.
ThirdPartyTest
--
A third party test shall test a scenario where a third party side is required
such as a JDBC driver or an external RDBMS instance.
CubridTest:
A CubridTest shall test scenarios where a Cubrid driver and/or external
instance is required.
Db2Test:
A DB2 test shall test scenarios where a DB2 driver and/or external instance is
required.
MainFrameTest:
A MainFrame test shall test scenarios where a MainFrame external instance is
required.
MysqlTest:
A MySql test shall test scenarios where a MySql driver and/or external instance
is required.
NetezzaTest:
A Netezza test shall test scenarios where a Netezza driver and/or external
instance is required.
OracleTest:
An Oracle test shall test scenarios where a Oracle driver and/or external
instance is required.
PostgresqlTest:
A PostgreSql test shall test scenarios where a PostgreSql driver and/or
external instance is required.
SqlServerTest:
An SqlServer test shall test scenarios where a SqlServer driver and/or external
instance is required.
KerberizedTest
--
A kerberized test shall run in kerberized environment thus it starts mini KDC
server.
These also can be refined and/or extended later but could be a good start for
now.
What do you think?
Thanks,
Bogi
- Boglarka
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68541/#review208172
---
On Aug. 28, 2018, 3:52 p.m., Nguyen Truong wrote:
>
> ---
> This is an automatically generated e-mail. To
