eclipse-m2e / m2e-core

Eclipse Public License 2.0
113 stars 116 forks source link

Migrate RCPTT test for MavenTargetDependencyEditor to SWTBot #1816

Closed ptziegler closed 1 month ago

ptziegler commented 3 months ago

In the long run, using RCPTT is probably causing more trouble than it's worth. It therefore makes sense to switch to a more lightweight solution like SWTBot for testing.

ptziegler commented 3 months ago

This also has the added benefit of allowing one to execute those tests directly within the Eclipse IDE.

github-actions[bot] commented 3 months ago

Test Results

  321 files  ± 0    321 suites  ±0   26m 42s :stopwatch: - 2m 48s   678 tests + 8    656 :white_check_mark: + 7  18 :zzz: ±0  3 :x: +1  1 :fire: ±0  2 034 runs  +24  1 976 :white_check_mark: +23  54 :zzz: ±0  3 :x: +1  1 :fire: ±0 

For more details on these failures and errors, see this check.

Results for commit fe3dadf3. ± Comparison against base commit aa4f8258.

This pull request removes 1 and adds 9 tests. Note that renamed tests count towards both. ``` DependencyEditorTest ``` ``` org.eclipse.m2e.pde.ui.MavenTargetDependencyEditorTest ‑ testAddMavenLocation org.eclipse.m2e.pde.ui.MavenTargetDependencyEditorTest ‑ testAddMavenLocationWithClipboard org.eclipse.m2e.pde.ui.MavenTargetDependencyEditorTest ‑ testColumnSorting org.eclipse.m2e.pde.ui.MavenTargetDependencyEditorTest ‑ testEditCellsDirectly org.eclipse.m2e.pde.ui.MavenTargetDependencyEditorTest ‑ testInitialButtonState org.eclipse.m2e.pde.ui.MavenTargetDependencyEditorTest ‑ testMultiSelection org.eclipse.m2e.pde.ui.MavenTargetDependencyEditorTest ‑ testRemoveArtifacts org.eclipse.m2e.pde.ui.MavenTargetDependencyEditorTest ‑ testUndoRedo org.eclipse.m2e.pde.ui.MavenTargetDependencyEditorTest ‑ testUpdateMavenArtifactVersion ```

:recycle: This comment has been updated with latest results.

HannesWell commented 3 months ago

Thank you @ptziegler for this migration, as said I have no idea about this part and trust your assessment. As long as the tests pass I'm happy with it. @laeubi or somebody else do you have any special interest in this?

Patrick, could you then revert the change from https://github.com/eclipse-m2e/m2e-core/pull/1163 regarding the formats of the tycho-p2-director-plugin? In fact the entire formats section can be removed since all values are the default then and with that actually the entire section for the tycho-p2-director-plugin in that pluginsManagement element is obsolete. Could you please remove that entirely in a second commit of this PR?

ptziegler commented 3 months ago

The instability seems to be gone. I need to make sure that the target platform has been resolved before executing the test. Otherwise the Maven target editor can't be opened reliably/in time.

In fact the entire formats section can be removed since all values are the default then and with that actually the entire section for the tycho-p2-director-plugin in that pluginsManagement element is obsolete

The entry itself needs to stay to specifiy the Tycho version, but the execution block is gone.

2024-08-31T15:25:59.5306983Z [WARNING] Some problems were encountered while building the effective model for org.eclipse.m2e:m2e-ide:eclipse-repository:2.0.0 2024-08-31T15:25:59.5310284Z [WARNING] 'build.plugins.plugin.version' for org.eclipse.tycho:tycho-p2-director-plugin is missing. 2024-08-31T15:25:59.5311392Z [WARNING] 2024-08-31T15:25:59.5312391Z [WARNING] It is highly recommended to fix these problems because they threaten the stability of your build.

akurtakov commented 2 months ago

Would you please look into failing tests (https://ci.eclipse.org/m2e/job/m2e/job/PR-1816/20/) ? And sorry for the merge commit (I wanted to rebase but misclicked!) please rebase on master and force push.

ptziegler commented 2 months ago

I think the initial approach of opening the Maven editor via the target-platform editor is just not very reliable... A lot depends on when the dependencies are resolved, which is effectively arbitrary. I now try to instantiate the Maven editor directly, which should overall be faster and more stable. This of course has the downside that this needs to know the actual classes and is therefore less agnostic.

ptziegler commented 2 months ago

All the SWTBot tests completed at least once now... The other failures are very likely unrelated.

akurtakov commented 1 month ago

Thanks for all the work. Merging.

HannesWell commented 1 month ago

Thank you @ptziegler!