[
https://issues.apache.org/jira/browse/HIVE-14536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15453943#comment-15453943
]
Peter Vary commented on HIVE-14536:
-----------------------------------
Hi [~kgyrtkirk], [~sseth],
I have to apologize here, since if everyone thought after HIVE-14444, that we
should refactor these classes only after the dust is settled, it was clearly my
lack of open source experience which caused problem here. You know, if everyone
travels on a highway opposite to me, clearly I must have chosen a wrong lane :)
To explain myself a little more, in my previous (numerous closed source)
projects, we usually finalized the basic concepts before adding new features,
non critical fixes and tuning performance, but starting to learn, that in open
source the "many baby steps" is a way to go.
Anyway, sorry for the misunderstanding again! I do my best, and learning as
fast as I can! :)
Some words for the changes in the patch:
I think there will be the following typical use cases for the qtest
infrastructure:
- Adding a new query - No differences here between the two solutions
- Adding a new test configuration
-- With the current solution we have to change minimum 2 files CliConfigs,
TestCase, and probably a driver
-- After the patch these changes are in one place, the TestCase
- Looking for a problem with a particular TestCase
-- With the current solution we go to the TestCase, follow the buildClassRule
method, realize, that it is defined by the Configuration, go to the
CliConfigurations, find the specific Adapter, find the method, and follow it to
the QTestUtil implementation - which means 4 redirection
-- After the patch from the TestCase you could go directly to the QTestUtil
implementation - immediate, straightforward
- Changing something in the QTestUtil, and checking which TestCases are
effected by the change
-- With the current solution we will find the affected Adapter easily, but
after that it is manual work, to find which TestCases are using that adapter
-- After the patch we could get direct link to the effected TestCases
While it is good to use the frameworks as such, but one have to be careful to
use the features which are beneficial in the current situation. Here we
absolutely need the parameterized test running, exactly as it has been
implemented, but in my opinion using rules not helps in our case. I think there
are many ok designs, like the current one, but our best solution would be the
following:
- Configuration - storing the test configurations - same as currently
- ConfigBuilder - creating the test configurations - as both of you pointed
out, and I implemented it
- TestCase - Defining the setup/before/after/shutdown functionality, especially
since some of the test cases need specific setup/cleanup procedures
- TestUtil(s) - Encapsulating the reoccurring functionality - for example there
are 4 kinds of test running and result evaluating methods used across the
TestCases, there is a typical functionality to remove created tables etc.
With this in mind, I still think we would have better, more maintainable
solution after this patch.
I hope I answered all your questions and comments regarding this patch. If not
so, or you do not agree with me, just write. I really hope, that I did not
blocked all work on the test cases - I have merged every affected change to
this one, and I am ready to do so with the following changes too, like
HIVE-14532, which I have already reviewed. Seriously! :) So do not hesitate to
work on them.
If we agree, that the general direction is good, and the patch needs only
incremental changes like the ConfigBuilder, I would be happy to invest the time
and the effort to make it happen. If you do not agree even with the basic
direction then please state it.
Thanks, and apologies again,
Peter
> Unit test code cleanup
> ----------------------
>
> Key: HIVE-14536
> URL: https://issues.apache.org/jira/browse/HIVE-14536
> Project: Hive
> Issue Type: Sub-task
> Components: Testing Infrastructure
> Reporter: Peter Vary
> Assignee: Peter Vary
> Attachments: HIVE-14536.5.patch, HIVE-14536.6.patch,
> HIVE-14536.7.patch, HIVE-14536.8.patch, HIVE-14536.patch
>
>
> Clean up the itest infrastructure, to create a readable, easy to understand
> code
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
