Skip to content

Make sure Launch Bar Manager sends event at a correct time #1486

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.

Sign up for GitHub

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

Draft
DangMinhTam382 wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Make sure Launch Bar Manager sends event at a correct time #1486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a052eb9
Make sure Launch Bar Manager sends event at a correct time
DangMinhTam382 Jun 25, 2026
019d665
Bump version
DangMinhTam382 Jun 25, 2026
aa7a299
Add 3 UT test cases that can detect original issue
DangMinhTam382 Sep 16, 2026
56e5ad4
Updated Unit test to make sure it cover all edge case
DangMinhTam382 Sep 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion launchbar/org.eclipse.launchbar.core.tests/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.launchbar.core.tests;singleton:=true
Bundle-Version: 1.0.100.qualifier
Bundle-Version: 1.0.200.qualifier
Fragment-Host: org.eclipse.launchbar.core;bundle-version="1.0.0"
Bundle-RequiredExecutionEnvironment: JavaSE-17
Require-Bundle: org.junit;bundle-version="[4.13.2,5)",
Expand Down
207 changes: 194 additions & 13 deletions ...se.launchbar.core.tests/src/org/eclipse/launchbar/core/internal/LaunchBarManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IConfigurationElement;
Expand All @@ -37,6 +38,7 @@
import org.eclipse.debug.core.ILaunchManager;
import org.eclipse.debug.core.ILaunchMode;
import org.eclipse.launchbar.core.DefaultLaunchDescriptor;
import org.eclipse.launchbar.core.ILaunchBarListener;
import org.eclipse.launchbar.core.ILaunchConfigurationProvider;
import org.eclipse.launchbar.core.ILaunchDescriptor;
import org.eclipse.launchbar.core.ILaunchDescriptorType;
Expand All @@ -45,6 +47,7 @@
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.osgi.service.prefs.BackingStoreException;

public class LaunchBarManagerTest {

Expand All @@ -56,12 +59,14 @@ public class LaunchBarManagerTest {
* <li>launchConfigTypeId = "fakeLaunchConfigType_no1";
* <li>launchDescTypeId = "fakeDescriptorType_no1";
* <li>preferredMode = "preferredMode_no1";
* <li>targetId = "fakeTargetId_no2"
* </ul>
*/
private static final String launchObject_no1 = "launchObject_no1";
private static final String launchConfigTypeId_no1 = "fakeLaunchConfigType_no1"; //$NON-NLS-1$
private static final String launchDescTypeId_no1 = "fakeDescriptorType_no1"; //$NON-NLS-1$
private static final String preferredMode_no1 = "preferredMode_no1"; //$NON-NLS-1$
private static final String targetId_no1 = "fakeTargetId_no1"; //$NON-NLS-1$
/**
* <p>This is a dummy launch object</p>
*
Expand All @@ -70,17 +75,25 @@ public class LaunchBarManagerTest {
* <li>launchConfigTypeId = "fakeLaunchConfigType_no2";
* <li>launchDescTypeId = "fakeDescriptorType_no2";
* <li>preferredMode = "preferredMode_no2"; // Not supported launch Mode
* <li>targetId = "fakeTargetId_no2"
* </ul>
*/
private static final String launchObject_no2 = "launchObject_no2";
private static final String launchConfigTypeId_no2 = "fakeLaunchConfigType_no2"; //$NON-NLS-1$
private static final String launchDescTypeId_no2 = "fakeDescriptorType_no2"; //$NON-NLS-1$
private static final String preferredMode_no2 = "preferredMode_no2"; //$NON-NLS-1$
private static final String targetId_no2 = "fakeTargetId_no2"; //$NON-NLS-1$

private static final String targetTypeId = "org.eclipse.launchbar.core.test.dummyLaunchTargetTypeId";//$NON-NLS-1$
private static final String runMode = "run"; //$NON-NLS-1$
private static final String debugMode = "debug"; //$NON-NLS-1$
private static final String attr_activeDesc = "attr_activeDesc";//$NON-NLS-1$
private static final String attr_activeTarget = "attr_activeTarget";//$NON-NLS-1$
private static final String attr_activeMode = "attr_activeMode";//$NON-NLS-1$

private LaunchBarManager launchBarManagerMock = null;
private ILaunchTargetManager targetManagerMock = null;
private ILaunchManager launchManagerMock = null;

@Before
public void before() throws CoreException {
Expand All @@ -90,7 +103,7 @@ public void before() throws CoreException {
ILaunchConfigurationType launchConfigType_no1 = createLaunchConfigType(launchConfigTypeId_no1,
preferredMode_no1, runMode, debugMode);
ILaunchConfigurationProvider launchConfigProvider_no1 = creatLaunchConfigProvier(launchConfigType_no1,
descriptor_no1, preferredMode_no1);
descriptor_no1, preferredMode_no1, targetTypeId);
launchConfigTypes.put(launchConfigTypeId_no1, launchConfigType_no1);
// Create mocked Object no2
ILaunchDescriptor descriptor_no2 = createLaunchDescriptorMock(launchObject_no2);
Expand All @@ -99,7 +112,7 @@ public void before() throws CoreException {
// preferredMode_no2 not supported
doReturn(false).when(launchConfigType_no2).supportsMode(preferredMode_no2);
ILaunchConfigurationProvider launchConfigProvider_no2 = creatLaunchConfigProvier(launchConfigType_no2,
descriptor_no2, preferredMode_no2);
descriptor_no2, preferredMode_no2, targetTypeId);
launchConfigTypes.put(launchConfigTypeId_no2, launchConfigType_no2);
// Mock Launch bar manager
List<IConfigurationElement> elements = new ArrayList<>();
Expand All @@ -110,21 +123,28 @@ public void before() throws CoreException {
elements.add(createConfigElementMockForConfigProvider(launchDescTypeId_no1, launchConfigProvider_no1));
elements.add(createConfigElementMockForDescriptorType(launchDescTypeId_no2, descriptor_no2));
elements.add(createConfigElementMockForConfigProvider(launchDescTypeId_no2, launchConfigProvider_no2));
ILaunchManager launchManager = createLaunchManagerMock(launchConfigTypes, preferredMode_no1, preferredMode_no2,
runMode, debugMode);
ILaunchTargetManager targetManager = createLaunchTargetManagerMock();
launchManagerMock = createLaunchManagerMock(launchConfigTypes, preferredMode_no1, preferredMode_no2, runMode,
debugMode);
targetManagerMock = createLaunchTargetManagerMock();
doReturn(elements.toArray(IConfigurationElement[]::new)).when(extension).getConfigurationElements();
// Mock Launch bar manager
launchBarManagerMock = createLaunchBarManagerMock(extensionPoint, launchManager, targetManager);
launchBarManagerMock = createLaunchBarManagerMock(extensionPoint, launchManagerMock, targetManagerMock);
// Initial LaunchBarManager
launchBarManagerMock.init();
}

@After
public void after() {
public void after() throws BackingStoreException {
launchBarManagerMock.getPreferenceStore().node(getNode(launchDescTypeId_no1, launchObject_no1)).clear();
launchBarManagerMock.getPreferenceStore().node(getNode(launchDescTypeId_no2, launchObject_no2)).clear();
launchBarManagerMock.getPreferenceStore().flush();
launchBarManagerMock.dispose();
}

private String getNode(String descTypeId, String descName) {
return String.format("%s:%s", descTypeId, descName);//$NON-NLS-1$
}

@Test
public void startupTest() throws Exception {
// Make sure the manager starts up and defaults everything to null
Expand Down Expand Up @@ -486,6 +506,119 @@ public void preferredLaunchModeTest_storedModeNotNull() throws CoreException {
assertEquals(preferredMode_no1, activeMode.getIdentifier());
}

/**
* <p>
* Test that when activeLaunchDescriptorChanged event is fired,launch bar has
* a stable/correct state.
* <p>
* Verifies that when "activeLaunchDescriptorChanged" event is fired, Launch bar
* manager has finished update launch mode and launch target for that descriptor
*
* @throws CoreException
*/
@Test

@betamaxbandit betamaxbandit Sep 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove InterruptedException?

@DangMinhTam382 DangMinhTam382 Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I removed them

public void launchBarStateTest_LaunchDescriptorChanged() throws CoreException {
// Dummy listener to collect data from the launch bar
AtomicReference<List<Map<String, Object>>> launchBarState = new AtomicReference<>();
launchBarState.set(new ArrayList<>());
ILaunchBarListener launchBarListener = createLaunchBarListener(launchBarState);
// Add initial active value to launch bar
ILaunchDescriptor desc_no1 = launchBarManagerMock.launchObjectAdded(launchObject_no1);
ILaunchTarget target_no1 = targetManagerMock.getLaunchTarget(targetTypeId, targetId_no1);
launchBarManagerMock.setActiveLaunchTarget(target_no1);
// Change active values on launch bar
launchBarManagerMock.launchObjectAdded(launchObject_no2);
ILaunchTarget target_no2 = targetManagerMock.getLaunchTarget(targetTypeId, targetId_no2);
launchBarManagerMock.setActiveLaunchTarget(target_no2);
launchBarManagerMock.setActiveLaunchMode(launchManagerMock.getLaunchMode(runMode));
// Add launch bar listener and start test
launchBarManagerMock.addListener(launchBarListener);
// Change active launch descriptor
launchBarManagerMock.setActiveLaunchDescriptor(desc_no1);
// Launch bar state when receive the event notification should be:
// launch descriptor: launchObject_no1
// launch target: targetId_no1
// launch mode: preferredMode_no1
for (var stateMap : launchBarState.get()) {
assertEquals(launchObject_no1, ((ILaunchDescriptor) stateMap.get(attr_activeDesc)).getName());
assertEquals(targetId_no1, ((ILaunchTarget) stateMap.get(attr_activeTarget)).getId());
assertEquals(preferredMode_no1, ((ILaunchMode) stateMap.get(attr_activeMode)).getIdentifier());
}
}

/**
* <p>
* Test that when activeLaunchTargetChanged event is fired,launch bar has
* a stable/correct state.
* <p>
* Verifies that when "activeLaunchTargetChanged" event is fired, Launch bar
* manager has finished update launch mode for that descriptor
*
* @throws CoreException
*/
@Test
public void launchBarStateTest_LaunchTargetChanged() throws CoreException {
// Dummy listener to collect data from the launch bar
AtomicReference<List<Map<String, Object>>> launchBarState = new AtomicReference<>();
launchBarState.set(new ArrayList<>());
ILaunchBarListener launchBarListener = createLaunchBarListener(launchBarState);
// Add initial active value to launch bar
launchBarManagerMock.launchObjectAdded(launchObject_no1);
ILaunchTarget target_no1 = targetManagerMock.getLaunchTarget(targetTypeId, targetId_no1);
launchBarManagerMock.setActiveLaunchTarget(target_no1);
// Change active values on launch bar
ILaunchTarget target_local = targetManagerMock.getLocalLaunchTarget();
launchBarManagerMock.setActiveLaunchTarget(target_local);
// Add launch bar listener and start test
launchBarManagerMock.addListener(launchBarListener);
// Change active launch target
launchBarManagerMock.setActiveLaunchTarget(target_no1);
// Launch bar state should be:
// launch descriptor: launchObject_no1
// launch target: targetId_no1
// launch mode: preferredMode_no1
for (var stateMap : launchBarState.get()) {
assertEquals(launchObject_no1, ((ILaunchDescriptor) stateMap.get(attr_activeDesc)).getName());
assertEquals(targetId_no1, ((ILaunchTarget) stateMap.get(attr_activeTarget)).getId());
assertEquals(preferredMode_no1, ((ILaunchMode) stateMap.get(attr_activeMode)).getIdentifier());
}
}

/**
* <p>
* Test that when activeLaunchModeChanged event is fired, launch bar has
* a stable/correct state.
* <p>
* Verifies that when "activeLaunchModeChanged" event is fired, all active
* fields in launch bar manager are up-to-date
*
* @throws CoreException
*/
@Test
public void launchBarStateTest_LaunchModeChanged() throws CoreException {
// Dummy listener to collect data from the launch bar
AtomicReference<List<Map<String, Object>>> launchBarState = new AtomicReference<>();
launchBarState.set(new ArrayList<>());
ILaunchBarListener launchBarListener = createLaunchBarListener(launchBarState);
// Add initial active value to launch bar
launchBarManagerMock.launchObjectAdded(launchObject_no1);
ILaunchTarget target_no1 = targetManagerMock.getLaunchTarget(targetTypeId, targetId_no1);
launchBarManagerMock.setActiveLaunchTarget(target_no1);
// Add launch bar listener and start test
launchBarManagerMock.addListener(launchBarListener);
// Change active mode
launchBarManagerMock.setActiveLaunchMode(launchManagerMock.getLaunchMode(runMode));
// Launch bar state should be:
// launch descriptor: launchObject_no1
// launch target: targetId_no1
// launch mode: Run
for (var stateMap : launchBarState.get()) {
assertEquals(launchObject_no1, ((ILaunchDescriptor) stateMap.get(attr_activeDesc)).getName());
assertEquals(targetId_no1, ((ILaunchTarget) stateMap.get(attr_activeTarget)).getId());
assertEquals(runMode, ((ILaunchMode) stateMap.get(attr_activeMode)).getIdentifier());
}
}

private ILaunchDescriptor createLaunchDescriptorMock(Object launchObject) throws CoreException {
ILaunchDescriptorType descriptorType = mock(ILaunchDescriptorType.class);
ILaunchDescriptor descriptor = mock(ILaunchDescriptor.class);
Expand All @@ -506,7 +639,7 @@ private ILaunchConfigurationType createLaunchConfigType(String launchConfigTypeI
}

private ILaunchConfigurationProvider creatLaunchConfigProvier(ILaunchConfigurationType launchConfigType,
ILaunchDescriptor desc, String preferredMode) throws CoreException {
ILaunchDescriptor desc, String preferredMode, String supportTargetTypeId) throws CoreException {
ILaunchConfigurationProvider configProvider = mock(ILaunchConfigurationProvider.class);
ILaunchConfiguration launchConfig = mock(ILaunchConfiguration.class);
doReturn(launchConfig).when(configProvider).getLaunchConfiguration(eq(desc), any(ILaunchTarget.class));
Expand All @@ -515,7 +648,8 @@ private ILaunchConfigurationProvider creatLaunchConfigProvier(ILaunchConfigurati
doReturn(launchConfig).when(desc).getAdapter(ILaunchConfiguration.class);
doAnswer(invocation -> {
ILaunchTarget target = (ILaunchTarget) invocation.getArguments()[1];
return target.getTypeId().equals(ILaunchTargetManager.localLaunchTargetTypeId);
return target.getTypeId().equals(supportTargetTypeId)
|| target.getTypeId().equals(ILaunchTargetManager.localLaunchTargetTypeId);
}).when(configProvider).supports(eq(desc), any(ILaunchTarget.class));
doReturn(preferredMode).when(configProvider).getPreferredLaunchModeId(eq(desc), any(ILaunchTarget.class));
return configProvider;
Expand All @@ -542,13 +676,26 @@ private IConfigurationElement createConfigElementMockForConfigProvider(String de

private ILaunchTargetManager createLaunchTargetManagerMock() {
ILaunchTargetManager targetManager = mock(ILaunchTargetManager.class);
ILaunchTarget localTarget = mock(ILaunchTarget.class);
doReturn(ILaunchTargetManager.localLaunchTargetTypeId).when(localTarget).getTypeId();
doReturn("Local").when(localTarget).getId(); //$NON-NLS-1$
doReturn(new ILaunchTarget[] { localTarget }).when(targetManager).getLaunchTargets();
ILaunchTarget localTarget = createLaunchTargetMock(ILaunchTargetManager.localLaunchTargetTypeId, "Local");//$NON-NLS-1$
ILaunchTarget dummyTarget_no1 = createLaunchTargetMock(targetTypeId, targetId_no1);
ILaunchTarget dummyTarget_no2 = createLaunchTargetMock(targetTypeId, targetId_no2);
doReturn(new ILaunchTarget[] { localTarget, dummyTarget_no1, dummyTarget_no2 }).when(targetManager)
.getLaunchTargets();
doReturn(localTarget).when(targetManager).getLocalLaunchTarget();
doReturn(localTarget).when(targetManager).getLaunchTarget(ILaunchTargetManager.localLaunchTargetTypeId,
"Local");
doReturn(dummyTarget_no1).when(targetManager).getLaunchTarget(targetTypeId, targetId_no1);
doReturn(dummyTarget_no2).when(targetManager).getLaunchTarget(targetTypeId, targetId_no2);
return targetManager;
}

private ILaunchTarget createLaunchTargetMock(String typeId, String id) {
ILaunchTarget localTarget = mock(ILaunchTarget.class);
doReturn(typeId).when(localTarget).getTypeId();
doReturn(id).when(localTarget).getId(); //$NON-NLS-1$
return localTarget;
}

private ILaunchManager createLaunchManagerMock(Map<String, ILaunchConfigurationType> launchConfigTypes,
String... supportModes) throws CoreException {
ILaunchManager launchManager = mock(ILaunchManager.class);
Expand Down Expand Up @@ -593,6 +740,40 @@ private ILaunchMode createLaunchModeMock(String identifier) {
return mode;
}

/**
* @param launchBarState variable to collect data
*/
private ILaunchBarListener createLaunchBarListener(AtomicReference<List<Map<String, Object>>> launchBarState) {
return new ILaunchBarListener() {
@Override
public void activeLaunchDescriptorChanged(ILaunchDescriptor descriptor) {
Map<String, Object> state = new HashMap<>();
state.put(attr_activeDesc, descriptor);
state.put(attr_activeTarget, launchBarManagerMock.getActiveLaunchTarget());
state.put(attr_activeMode, launchBarManagerMock.getActiveLaunchMode());
launchBarState.get().add(state);
}

@Override
public void activeLaunchModeChanged(ILaunchMode mode) {
Map<String, Object> state = new HashMap<>();
state.put(attr_activeDesc, launchBarManagerMock.getActiveLaunchDescriptor());
state.put(attr_activeTarget, launchBarManagerMock.getActiveLaunchTarget());
state.put(attr_activeMode, mode);
launchBarState.get().add(state);
}

@Override
public void activeLaunchTargetChanged(ILaunchTarget target) {
Map<String, Object> state = new HashMap<>();
state.put(attr_activeDesc, launchBarManagerMock.getActiveLaunchDescriptor());
state.put(attr_activeTarget, target);
state.put(attr_activeMode, launchBarManagerMock.getActiveLaunchMode());
launchBarState.get().add(state);
}
};
}

// TODO - test that changing active target type produces a different launch
// config type
// TODO - test that settings are maintained after a restart
Expand Down
2 changes: 1 addition & 1 deletion launchbar/org.eclipse.launchbar.core/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.launchbar.core;singleton:=true
Bundle-Version: 3.2.0.qualifier
Bundle-Version: 3.2.100.qualifier
Bundle-Activator: org.eclipse.launchbar.core.internal.Activator
Bundle-Vendor: %providerName
Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.34.0,4.0.0)",
Expand Down
4 changes: 2 additions & 2 deletions .../org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/internal/LaunchBarManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,12 +496,12 @@ public void setActiveLaunchDescriptor(ILaunchDescriptor descriptor) throws CoreE
doSetActiveLaunchDescriptor(descriptor);
// store in persistent storage
storeActiveDescriptor(activeLaunchDesc);
// Send notifications
fireActiveLaunchDescriptorChanged();
// Set active target
syncActiveTarget();
// Set active mode
syncActiveMode();
// Send notifications
Comment thread
betamaxbandit marked this conversation as resolved.

@betamaxbandit betamaxbandit Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also changes the observable ordering of ILaunchBarListener notifications: target/mode notifications may now occur before the descriptor notification. I think that's reasonable for this fix, but because ILaunchBarListener is public API it would be useful for the regression test to make the intended semantics clear, in particular that listeners should rely on the manager state being consistent rather than on descriptor/target/mode notification ordering.

@DangMinhTam382 DangMinhTam382 Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Mr. @betamaxbandit ,

listeners should rely on the manager state being consistent rather than on descriptor/target/mode notification ordering.

Agree! This is what this change is aiming for.
Whenever an event is fired, launch bar manager is expected to have up-to-date fields.
I added 3 UT to cover for this.

fireActiveLaunchDescriptorChanged();

@Kummallinen Kummallinen Jun 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just moving this doesn't really solve the issue, the events for target & mode are still sent too early. This was why I suggested we create a new event that was for when any of them changed then we can just fire that once

@DangMinhTam382 DangMinhTam382 Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment, Mr. @Kummallinen

About your suggestion, I did spend quite some time testing it out.
Tried posting a "LaunchBarChanged" event at the very end of these 3 methods: "setActiveLaunchDescriptor()", "setActiveLaunchMode()", "setActiveLaunchTarget()".
And it worked.

Then I realized that existing events "activeLaunchTargetChanged" & "activeLaunchModeChanged" are sent at the same point as the newly added event. The only exception is "activeLauchDescriptorChanged", which is fired too early.
With the changes in this commit, all three existing events and the new event are sent at the same time.

From an event-handling perspective, there is no noticeable difference between the existing events and the new event. That's why I decided to go with the approach in this commit.

--
About your concern:

the events for target & mode are still sent too early

Since "syncActiveTarget()" and "syncActiveMode()" is calling to "setActiveLaunchMode()" and "setActiveLaunchTarget()", respectively, the change events would still be fired too early even with the new event.

-> Possible suggestion to fix for this would be creating internal method that does NOT fire event,
Ex:
"syncActiveTarget()" -> "internalSetActiveLaunchTarget()" -> "internalSetActiveLaunchMode()"
However, this would be a much larger change and could potentially impact existing ISV implementations that relies on the current event behavior.

Therefore, instead of trying to ensure that events are fired at desired time, this commit focuses on ensuring that all Launch Bar components are up to date when a change event is fired/caught, even if the triggering method has not yet finished executing.

@betamaxbandit betamaxbandit Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi I am ok with this change.
However, could we add a regression test for this change? The bug is specifically about the state visible to a listener when activeLaunchDescriptorChanged() is called. I think the test should switch to a descriptor which results in a different target/mode and, from inside activeLaunchDescriptorChanged(), assert that getActiveLaunchTarget() and getActiveLaunchMode() already contain the values for the new descriptor. That should fail with the old ordering and protects the exact behaviour being fixed.

Also could you detail somewhere what other testing should be performed please? Is there a failing test at the moment that shows the problem obviously?

@DangMinhTam382 DangMinhTam382 Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Mr. @betamaxbandit ,

Is there a failing test at the moment that shows the problem obviously?

ATM, there is no test case that could detect this issue.

I think the test should switch to a descriptor which results in a different target/mode and, from inside activeLaunchDescriptorChanged(), assert that getActiveLaunchTarget() and getActiveLaunchMode() already contain the values for the new descriptor. That should fail with the old ordering and protects the exact behaviour being fixed.

Thanks for the suggestion.
I added 3 UT that could detect this issue and should be enough to cover for future changes relate to event firing order.
Test for 3 events:

  • activeLaunchDescriptorChanged
  • activeLaunchTargetChanged
  • activeLaunchModeChanged

}

private void doSetActiveLaunchDescriptor(ILaunchDescriptor descriptor) {
Expand Down
Loading