On Tue, Nov 17, 2009 at 16:14, Carsten Ziegeler <cziege...@apache.org> wrote: > But we have a problem in the slingtest module - this module uses > internal classes of the jcr resource module - which it definitly should > not; internal classes should imho not be used by any other library. And > this stuff is exactly the stuff that creates the circular dependency. > So I'm personally more in favour of removing this again! > We have one place where it is used (in the i18n tests) and I think we > can change the test easily by using mock objects.
The jcr resource resolver used must be a real one and not a mock object! The thing about the SlingTestHelper is that it makes your life easier by working in a way not requiring an osgi framework running in the unit test. That requires some tricks (incl. access to internal packages), because configuration for most objects can normally only be provided via osgi config. Same for the AdapterManagerTestHelper (still in commons/testing, but part of the SlingTestHelper code from the SLING-1166 patch). A better solution (w/o requiring unit tests to run a osgi framework) could be that those bundles make such a usage simpler, w/o having to depend on internal classes. But using internal packages for unit testing is IMO ok (and merely a blocker, because IDE test runners such as in Eclipse or the maven surefire plugin use the full jars for the classpath anyway and do not use the osgi bundle header). Also, these tests are intended to be used by non-Sling code. I moved them over from our product (cq5) where they are used for quite a few unit tests. The unit tests still use the original code in the product, but I wanted to switch over to the Sling variant and drop that old code. > So my suggested actions are now: > - remove dependency to internal stuff (and adapt the tests) -1 as explained, this is not possible (AFAICS) > - merge both testing modules into a single one > - move the testing module out of commons I would keep them split. The question is what commons.testing is about? Is it mainly for making it easier to write tests for sling core bundles? In that case we definitely have a circular dependency and must keep both split. Otherwise we can merge them and move them out. The SlingTestHelper itself should never grow to have more dependencies than just that of the (few) core bundles. Regards, Alex -- Alexander Klimetschek alexander.klimetsc...@day.com
