Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [egit-dev] Re: [master] Change I5828502f: (egit/parallelip-jgit) Change jgit build to be completely manifest first

On Tue, Dec 22, 2009 at 8:27 PM, Shawn O. Pearce <spearce@xxxxxxxxxxx> wrote:
>> I had a very quick look. You have the same package, say
>> org.eclipse.jgit.treewalk, defined both in org.eclipse.jgit and
>> org.eclipse.jgit.test bundles. You also use Import-Package to wire
>> org.eclipse.jgit and org.eclipse.jgit.test together. Normally, there is
>> only one producer of each package, so Import-Package gets resolved to
>> org.eclipse.jgit, which effectively hides test classes at runtime.
>
> Wow.  Thanks for figuring that out, I am apparently a complete moron.
>
> I think we do want our tests declared in the same package as the
> code they test, sometimes we use package visibility to help test
> internals, but we never want to expose that internal to a public
> API consumer because we might break the interface in the future.

In OSGi, the general approach is to not have your tests declared in
the same package. You generally have a bundle that contains your tests
(which sometimes is a fragment), and follows the naming convention of
the bundle. For example, org.eclipse.jgit.test is the bundle, than you
would have packages org.eclipse.jgit.test.{treewalk, etc...}. At
Eclipse, we have a policy of exporting all packages. If they are
internal, they get marked as x-internal: true or as an x-friends if
there is a particular bundle that has special access to the internals.
I think the simple fix here is to refactor the packages in the test
bundle.

>> Also, keep in mind that bundle classloader.getResource and similar
>> methods return bundleentry:// urls, but there are at least few places in
>> the tests where you do "new File(cl.getResource(...).toURI())". This
>> will have to change to run in OSGi runtime.

These are relatively simple fixes and can be fixed if I see the failures ;)

Cheers,

-- 
Chris Aniszczyk | EclipseSource Austin | +1 860 839 2465
http://twitter.com/eclipsesource | http://twitter.com/caniszczyk


Back to the top