Hi Andrei, On Tue, Apr 26, 2016 at 3:53 PM, Andrei Dulvac <andrei.dul...@gmail.com> wrote: > ...The sling-testing-tools [0] has become a container for different small > pieces of code that don't fit in the other projects under testing/ [1]...
I agree that that module is not consistent, it contains unrelated classes as you mention. > The changes would be the following: > * leave the tools/ (sling testing tools) untouched, with the idea to > eventually deprecate it if the other modules fit the needs... Sounds good, but we'd have to be careful to actually do this and deprecate that module as soon as we're happy with the alternatives that you suggest. In general I like the next modules that you suggest, but IMO any such patches should come with additional patches that demonstrate how this new stuff can be used in existing or new tests in our core codebase, and how it makes our tests nicer. > * create a new module, called serversetup with the jarexec and serversetup > packages copied from sling-testing-tools. sounds good > * create module http/clients with some stuff like the WebconsoleClient > copied from tools/ and a bunch of classes that replace the RequestExecutor > and client with Threadsafe, well-tested and HttpClient-compatible http > tooling... sounds good as well in general, but I'm not sure if everything belongs there in your branch. AbstractPoller for example looks independent from HTTP clients. The clients/html stuff also looks relatively independent. There's clients/quickstart, but we don't have a quickstart in Sling, should this have a better name? Does it really belong in there? clients/html/microdata also looks fairly specific. All in all it looks like there's a lot in that http/clients module, I'm wondering if it all makes sense for the Sling core (as /testing is part of that) or if some of that is too specific, and also if that should be broken up into smaller, more specific modules. Some of which might to under /contrib instead. >...create module http/rules with junit tooling dependent on the clients > which follow the more modern approach to junit rules rather than test base > classes and provide ootb rules with clients, junit filtering, configuring > references to available sling instances,... There's lots of cool stuff in there but most of it doesn't look specifically related to HTTP. Should that module also be broken up in smaller pieces? Or can it be made more minimalistic by removing some rules that are specific to your way of working but might not be relevant for Sling as a whole and could move to /contrib instead? Don't get me wrong, I like your idea of cleaning up testing tools very much. I'm just wary of introducing new somewhat big and catch-all modules, I'd prefer whatever goes under /testing to be as lean as possible, some non-core classes and rules might move to contrib to reduce the size of what needs to be maintained at core level. -Bertrand
