On Fri, Feb 07, 2014 at 10:19:18AM -0500, Chris Evich wrote: > On 02/05/2014 12:07 PM, Lucas Meneghel Rodrigues wrote: > > (3) - Somehow come up with 'rules' about what belongs to test providers > > -virttest -autotest. > > (4) - Have API police ready to enforce the rules. > > Suggestions: > > Perhaps ignore the module level at first. It's an organization layer > developed within a different context then we're after here. The heart > of the matter really is what functions and classes are there, what are > their relationships, and where should they go based on their current > purpose and relationships. So: > > 3a) Classes/Functions which are clearly serving all test-providers > belong in the core. That does not include items which have _potential_ > or _intended_ future uses, only actual and current. > > 3b) Classes/Functions which are less-clear, out-of-place, or only > marginally serve one TP while substantially serving another should be > "adjusted" to fit #3a. i.e. move/duplicate the minority use-case > elsewhere, fold it up the dependency tree into it's caller(s), etc. > > 4a) Anyone doing code-review needs to keep an eye out for #3, especially > for additions to the core code. It's _MUCH_ easier to move code from > a TP into core (due to some future need) than the other way around (as > we are witnessing).
So far so good. > > 4b) Unittests for core code must not depend on system (i.e. external > tools, configuration, or data) -or- repository state (i.e. higher-level > module availability or present TP code/tests). These are key to early > organizational problem detection. Agree that core could should never *ever* depend on TP code. > > 4c) TP unittests on the other hand, _SHOULD_ make liberal use of > available code-code and unittests. However, dependence on system state > is still forbidden. These are also key to early organizational problem > detection. As long as there are no TP interdependencies, yes. But for both 4b and 4c, I'm not sure what do you mean by system state and external tools. Sometimes a test becomes very complex and expensive if you have to "mock" everything up. So depending on external configuration/tools/state is acceptable, as long as the test is disabled by default and properly documented. Most of the time, this is better than no test at all. > > 4d) If/when #3 is violated due to review or mistake, the original author > is responsible. That implies the consequence is in fixing all > subsequent commits piled on top before detection. i.e. this could mean > re-working a LOT of other people's layered commits. > > 4e) If original author is not available, the re-write responsibility > falls to the reviewer(s), then it goes up the chain from there. Although in practice I think what you've described will happen naturally, I suggest we leave it out of the "rules" or "policy" for now. It's probably obvious enough (and I'm a big fan of the KISS principle). Let's see how the community behaves and see if this issue turns out to be a problem or not. Otherwise you solve a potential problem (violation of #3) but create another one (original author or reviewer not available, or busy doing something else). Thanks. - Ademar -- Ademar de Souza Reis Jr. Red Hat ^[:wq! _______________________________________________ Virt-test-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/virt-test-devel
