-
Notifications
You must be signed in to change notification settings - Fork 113
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
Call the "activated" method of the default tab in a Launch Config Dialog #860
Call the "activated" method of the default tab in a Launch Config Dialog #860
Conversation
3aeb7d9
to
3717efa
Compare
Please consider that there already exist So either new tests should be added there as well or creating a |
3717efa
to
80acc22
Compare
@iloveeclipse since you created |
Also, I'm puzzled about the errors. I see these 3 errors in the Verify Windows step: 1: Problems in
|
80acc22
to
1411bd2
Compare
@akurtakov are these instructions up to date: https://wiki.eclipse.org/Platform-releng-faq#What_needs_to_be_done_to_add_a_new_test_bundle? I noticed you worked on them back in February 2022 |
Yes, please do not create new bundle. Historically org.eclipse.debug.tests contain both core/ui tests, you can see there are console tests, memory view tests etc.
This is known tycho issue eclipse-tycho/tycho#3019 where tycho now insists that build.properties contain an extra entry for bin directory (previously this wasn't needed). The workaround is to add that entry in the build.properties of affected bundle(s). |
They used to be back then. I can't remember anything big that changed so it should be correct. |
You can find the log file in the according Jenkins run. The log states that there are warnings in the added code, which are treated as errors:
|
1411bd2
to
92b20bb
Compare
a2cdf99
to
4e96983
Compare
@fedejeanne : I've pushed fixed tests, please check. |
@fedejeanne @iloveeclipse |
4e96983
to
a3f81d6
Compare
Done
I haven't seen any changes in the tests, where did push exactly? FWIW the tests run locally and they seem to run in GH too since all checks have passed. This PR looks ready to be merged, wouldn't you agree? |
On your github branch. Unfortunately you discarded them.
No, see my comment above:
|
@fedejeanne The commit is still there: 4e96983 |
Oh, sorry, I must have force-pushed before I got your message. Care to push again or are the problems gone/fixed now?
I added another PR for that one since I think it's unrelated to this PR --> #862. It's looking good, thank you for the hint! |
You can just compare with 4e96983 to see the changes and integrate them again. |
Thanks! I just compared them and saw that Andrey added the test class to the suite. I just happened to add those same changes in a3f81d6 |
...clipse.debug.tests/src/org/eclipse/debug/tests/ui/LaunchConfigurationTabGroupViewerTest.java
Outdated
Show resolved
Hide resolved
...clipse.debug.tests/src/org/eclipse/debug/tests/ui/LaunchConfigurationTabGroupViewerTest.java
Outdated
Show resolved
Hide resolved
...clipse.debug.tests/src/org/eclipse/debug/tests/ui/LaunchConfigurationTabGroupViewerTest.java
Show resolved
Hide resolved
...clipse.debug.tests/src/org/eclipse/debug/tests/ui/LaunchConfigurationTabGroupViewerTest.java
Show resolved
Hide resolved
...ui/org/eclipse/debug/internal/ui/launchConfigurations/LaunchConfigurationTabGroupViewer.java
Outdated
Show resolved
Hide resolved
...ui/org/eclipse/debug/internal/ui/launchConfigurations/LaunchConfigurationTabGroupViewer.java
Outdated
Show resolved
Hide resolved
...ui/org/eclipse/debug/internal/ui/launchConfigurations/LaunchConfigurationTabGroupViewer.java
Outdated
Show resolved
Hide resolved
484c538
to
c35bbfd
Compare
Also add a new test bundle that contains the proper regression test. Bundle: org.eclipse.debug.ui.tests Class: LaunchConfigurationTabGroupViewerTest This class also contains a regression test for eclipse-platform#766 Fixes: eclipse-platform#859
c35bbfd
to
2fe8228
Compare
Is this one ready, if yes please merge |
This PR unblocked eclipse-pde/eclipse.pde#946 fyi @HannesWell |
This PR adds a fix (#859) and also a new test bundle that contains:
FWIW I followed these instructions to add the new test bundle (If they are outdated, please let me know) and I also added the new bundle as a
<module>
in thepom.xml
.Fixes: #859
Contributes to: eclipse-pde/eclipse.pde#674