-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an example that could be used inside PDE #503
base: main
Are you sure you want to change the base?
Conversation
1d62a20
to
f6a9b4a
Compare
Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we test this in our CI builds? The other examples focus on command line builds (gradle, maven). This PR includes classpath files which are brittle (they require JDK 11).
This would be more interesting if it were a maven tycho build which could be verified by CI and we can keep up to date. I am not interested in this in its current state.
It is an example for usage in the with the Eclipse PDE tooling and not a commandline / ci.
Whats wrong with JDK 11? Other examples require bnd, gradle ...
then probably you should in the next eclipse-con presentation not state that "osgi-test should work with PDE (note: this has not been tested by us)" and instead write "but we are not interested and don't care" ...
Tycho can build it but has nothing to do with PDE as tycho does not use PDE... but feel free to came up with a PDE build I just have no clue how to use it and don't know anyone still using it... |
Yes, I understand that. This is why I don't think it is a good idea as we cannot continuously test to make sure it still works.
Nothing per se, but the example does not work unless the user has JDK 11 installed into their Eclipse configuration. And when they don't, they get a lot red errors in Eclipse and no real explanation as to why.
It is not that we do not care, but if you ship something, you should have confidence it works. With no way to test, on a continuous basis, that the example works, the example will eventually cease to be valid. For example, I have no idea that the example in the PR even works right now. It could be broken. I would need to check out this PR, install Eclipse PDE and try to load it (I hope I have JDK 11 configured into that Eclipse installation...). With a CI build, this PR would be able to build and test the example to confirm it does in fact work.
I know that Tycho is used to build Eclipse which can also be built from the PDE information. So there must be some relationship. I don't know Tycho either nor do I use PDE. But I do think we must be able to build the code in the repo in our CI build. |
Recent eclipse needs at least Java 11 and the classpath reference only the profile JavaSE-11 that could be mapped to any JVM with version 11+ so you only run into problem when you remove all but a JDK8 ... that's very uncommon setup I think...
It still is useful to prove it does work in an PDE also, if it might fail in the future one probably has to adjust it, but having no (manual) test case at all is even worse I think... but in the end its of course the choice of the project, we can simply keep this PR open just in case someone asks for an (continuous untested) example.
Yes but tycho only uses the information from PDE but has completely different code when it comes to executing tests (most of that parts are pluggable and require a running UI in PDE/Eclipse).
Integrate this in a CI build would require a lot of work (e.g xvnc server, complete eclipse product launch, probably something like RCPTT and that's beyond what I like to invest here at the moment given that it probably would still not be accepted by (to me) unknown project rules. |
@laeubi Would it be possible to update this example to be a Maven/Tycho example? Then (I assume), the example could be used in PDE and we can also run the maven build in the CI build to confirm the example is correct. |
I'll give it a try, it just should be taken into account that PDE does not use Tycho so actually if it works on Tycho ist not a proof it will work in the PDE as well but I think thats probably acceptable. |
@laeubi are you plan on doing something here? This might be a good point for an example with the new bnd pde hybrid? |
I plan to improve OSGI-Testing support in PDE, but if you aim for |
This is an example where one can run osgi-test from inside Eclipse PDE
To execute it do the following:
Run As
>Junit Plugin Test