Apply the -runbundles+ decorator on the computed RunBundles when resolving #6407
Conversation
…to the runbundles resolver output as well. Signed-off-by: Arnoud Glimmerveen <[email protected]>
| @@ -208,7 +210,16 @@ public boolean updateBundles(BndEditModel model) { | |||
| newer = older; | |||
| } | |||
|
|
|||
| model.setRunBundles(newer); | |||
| // Apply the -runbundles decorator on the computed RunBundles | |||
There was a problem hiding this comment.
You have to change the check if the runbundles are identical before on line 197. The decoration might change the runbundles.
There was a problem hiding this comment.
I have moved the application of decoration above the older/newer check.
| @@ -0,0 +1,7 @@ | |||
| invoker.goals=--no-transfer-progress package | |||
There was a problem hiding this comment.
Hmm. I am not too happy this testing takes place in the maven sub project. The functional change is in bnd so it should be tested in the normal bnd testing. The biz.aQute.resolve.RunResolutionTest in biz.aQute.resolve contains examples.
I know it is more comfortable to stay in your comfort zone but bnd is a shared project. If you change a function in bnd the test must take place as close as possible to the change.
There was a problem hiding this comment.
That makes sense, I will look into adding test case(s) in RunResolutionTest. Do you want to retain these maven sub project test as well for this particular case, or just the tests within biz.aQute.resolve?
There was a problem hiding this comment.
I think it is better to test it once in bnd. I consider ANY error that would be detected in a 'driver' (maven, gradle, eclipse, intellij) an error of the highest severity. We rarely have them fortunately. And in this case, the functionality is clearly in bnd, not the driver.
There was a problem hiding this comment.
Ok, I have removed the integration tests from the maven sub module.
* Moved decoration between older/newer check to ensure that check also incorporates the decoration. * Added JUnit test. Signed-off-by: Arnoud Glimmerveen <[email protected]>
|
I have pushed a commit that should address the feedback. The only question I have is whether the tests in de maven sub module should be retained. |
…tionally covered by RunResolutionTest Signed-off-by: Arnoud Glimmerveen <[email protected]>
|
thanks! |
This PR should address issue #6364 . I applied the same mechanism used within Project.getBundles in the RunResolution's updateBundles function, just before the model is updated with the new RunBundles.