fix: update test icons to account for bundling, fix apexDB location, and fix checkpoint issue #4686

gbockus-sf · 2023-02-16T06:49:35Z

What does this PR do?

Primarily this PR addresses fixing accessing files in the resources directory for bundled vsixes.

Icons shows in the lwc and Apex Test views.
Setting up the apexDb

Secondarily there was a simple fix to how checkpoints are handled so I just did the update here.

What issues does this PR fix or reference?

@W-12545057@ #4534
@W-12545057@ #4673

Functionality Before

No icons would show up in the Test views.
checkpoints would be removed out of order in the checkpoints view.

Functionality After

We have icons.

checkpoints are correctly removed from the debug panel when toggle in a cls.

Notes:
To exercise the checkpoints functionality you can toggle a checkpoint via the SFDX: Toggle Checkpoint command in the CP or by creating a breakpoint and setting the expression to 'checkpoint'. Checkpoints can also be removed by clicking an active checkpoint (breakpoint).

gbockus-sf · 2023-02-16T06:50:18Z

package.json

@@ -82,7 +82,7 @@
    "vscode:bundle:debugger": "lerna run bundle:debugger",
    "vscode:bundle:extension": "lerna run bundle:extension",
    "vscode:package": "npm run vscode:bundle:debugger && npm run vscode:bundle:extension && lerna run vscode:package --concurrency 1 && npm run reformat",
-    "vsix:install": "find ./packages -name '*.vsix' -exec code --install-extension {} \\;",
+    "vsix:install": "find ./packages -name '*.vsix' -exec code-insiders --install-extension {} \\;",


if I'm the only one using this for vsix testing figured I'd update instead of always editing it.

Hmm - maybe we could have two? vsix:install and vsix:install:insiders?

gbockus-sf · 2023-02-16T06:51:04Z

packages/salesforcedx-utils-vscode/src/helpers/extensionUris.ts

+  return vscode.Uri.joinPath(baseUri, relativePath);
+}
+
+export const extensionUris = {


In the spirit of always handing Uri's the same way created a common helper in utils to handle repeatable tasks. Think pathUtils but for Uris.

gbockus-sf · 2023-02-16T06:51:19Z

packages/salesforcedx-utils-vscode/src/helpers/paths.ts

+  const relativePathToToolsFolder = path.join(
+    projectPaths.relativeStateFolder(),
+    TOOLS
+  );


format 🤷

gbockus-sf · 2023-02-16T06:52:15Z

packages/salesforcedx-vscode-apex-replay-debugger/src/breakpoints/checkpointService.ts

@@ -808,14 +808,14 @@ export async function processBreakpointChangedForCheckpoints(
  for (const bp of breakpointsChangedEvent.removed) {
    if (bp.condition && bp.condition!.toLowerCase().indexOf(CHECKPOINT) >= 0) {
      await lock.acquire(CHECKPOINTS_LOCK_STRING, async () => {
-        const breakpointId = (bp as any)._id;
+        const breakpointId = bp.id;


this was the fix for the breakpoints panel. I'm not sure when this changed in the vscode spec, but def calls to light not referencing private properties in 3rd party libs as a best practice.

gbockus-sf · 2023-02-16T06:52:53Z

...dx-vscode-apex-replay-debugger/test/vscode-integration/checkpoints/checkpointService.test.ts

@@ -209,7 +208,7 @@ describe('Verify checkpoint callback for vscode.debug.onDidChangeBreakpoints', (

    // Verify that a single checkpoint has been added to the checkpoint service
    const theNode = checkpointService.returnCheckpointNodeIfAlreadyExists(
-      breakpointId
+      breakpoint.id


the id value is readonly so we were unable to manipulate the breakpoint Id as was done previously. I just updated to use the existing id and it worked

gbockus-sf · 2023-02-16T06:53:18Z

packages/salesforcedx-vscode-apex/jest.config.js

+// eslint:disable-next-line:no-var-requires
+const baseConfig = require('../../config/jest.base.config');
+
+module.exports = Object.assign({}, baseConfig);


first jest test in vscode-apex package.

gbockus-sf · 2023-02-16T06:55:18Z

packages/salesforcedx-vscode-apex/src/constants.ts


 export const DEBUGGER_LINE_BREAKPOINTS = 'debugger/lineBreakpoints';
 export const DEBUGGER_EXCEPTION_BREAKPOINTS = 'debugger/exceptionBreakpoints';

-export const LIGHT_BLUE_BUTTON = path.join(


So the issue with the icons was the code assumed a location of the source and found the resources directory from the current file location. That doesn't work with bundling b/c the source moves from this dir to the minified js file. I created a new class to handle it in a sane way instead of trying to hack it together as a constant value.

gbockus-sf · 2023-02-16T06:55:58Z

packages/salesforcedx-vscode-apex/src/helpers/languageServerUtils.ts

+import * as vscode from 'vscode';
+import { VSCODE_APEX_EXTENSION_NAME } from '../constants';
+
+const setupDB = (): void => {


Just refactored this out of the file of the class that uses it to make it easier to unit test.

gbockus-sf · 2023-02-16T06:57:16Z

packages/salesforcedx-vscode-apex/src/helpers/languageServerUtils.ts

+      unlinkSync(dbPath);
+    }
+
+    try {


Same story as the icons logic wise. This was assuming previously it's location in the file system to find the resources directory ala const systemDb = path.join(__dirname, '..', '..', 'resources', 'apex.db');
I updated it to use the extensionUri method to find the extension directory which works for both dev and published versions.

gbockus-sf · 2023-02-16T06:58:43Z

packages/salesforcedx-vscode-apex/src/views/icons/IconsEnum.ts

+  DARK_RED_BUTTON = 'DARK_RED_BUTTON',
+  DARK_GREEN_BUTTON = 'DARK_GREEN_BUTTON',
+  DARK_ORANGE_BUTTON = 'DARK_ORANGE_BUTTON'
+}


So my goal here was to make using this icons in code a simple and type safe thing. Will break it down in the helper file.

gbockus-sf · 2023-02-16T07:00:38Z

packages/salesforcedx-vscode-apex/src/views/icons/iconHelpers.ts

+ * @param key A key from the {@link IconsEnum}
+ * @returns The Uri to the icon image.
+ */
+const getIconPath = (key: iconKey) => {


So we needed a way to get iconPaths. My first approach here was to initialize with the extensionContext so I could get the extensionPath. That lead to some ugliness due to the file using this being initialized early in the activation flow. To avoid race condition games and have to store the extensionContext here I altered this to use the extensionUri to get the base extension location.

gbockus-sf · 2023-02-16T07:01:47Z

packages/salesforcedx-vscode-apex/src/views/icons/iconHelpers.ts

+  const baseExtensionPath = extensionUris.extensionUri(
+    VSCODE_APEX_EXTENSION_NAME
+  );
+  const iconUri = extensionUris.join(baseExtensionPath, ICONS[key]);


Then using that Uri and the relative path to the icon I join them and return that Uri to be used by the outline provider.

Note that the outlineProvider implements a vscode tree view so the iconPaths are typed to Uris or strings.

gbockus-sf · 2023-02-16T07:04:35Z

packages/salesforcedx-vscode-apex/src/views/icons/iconHelpers.ts

+ * Get the Uri for an icon located in the resources directory.
+ * @param key A key from the {@link IconsEnum}
+ * @returns The Uri to the icon image.
+ */


Note the param here is limited to the key values from the Icons const (which are also the keys for the IconsEnum).
This allows us to use this method safely always limiting the param to the available icons (or getting a compile error)

gbockus-sf · 2023-02-16T07:06:46Z

packages/salesforcedx-vscode-apex/src/views/testOutlineProvider.ts

@@ -368,4 +359,11 @@ export class ApexTestNode extends TestNode {
  public contextValue = 'apexTest';
 }

-export const testOutlineProvider = new ApexTestOutlineProvider(null);


So this was leading to all kinds of sadness trying to both write the tests and ensure when this ApexTestOutputProvider was being constructed. I don't love the poor substitute for a singleton, but I would have had to majorly refactor the int tests if I altered how this property is referenced.

This seemed like the lesser of two evils path but I'm open to suggestions.

This looks like a solid solution to avoid the big refactor to me.

gbockus-sf · 2023-02-16T07:07:39Z

packages/salesforcedx-vscode-lwc/src/testSupport/testExplorer/iconPaths.ts

-const extensionPath = path.join(__filename, '..', '..', '..', '..', '..');
-const LIGHT_BLUE_BUTTON = path.join(
+const extensionPath = extensionUris.extensionUri(VSCODE_LWC_EXTENSION_NAME);
+const LIGHT_BLUE_BUTTON = extensionUris.join(


The LWC constants were setup to be more easily updated so I went the easy path here instead of having two identical icon helpers.

gbockus-sf · 2023-02-16T07:08:10Z

...es/salesforcedx-vscode-lwc/test/vscode-integration/testSupport/testExplorer/testNode.test.ts

-        true
-      );
+      expect(
+        groupNode.iconPath.dark.toString().endsWith('testNotRun.svg')


These updates are b/c the icon is now a Uri instead of a string path.

packages/salesforcedx-utils-vscode/test/jest/helpers/extensionUris.test.ts

klewis-sfdc · 2023-02-16T20:38:30Z

These code changes look great. Thanks for the update to make icons much more easy to work with!

I left a question re the script and a super small suggested fix. And it looks like the integration tests need to be tweaked but this looks really good.

randi274 · 2023-02-16T22:32:32Z

packages/salesforcedx-vscode-apex/src/views/testOutlineProvider.ts

@@ -280,20 +271,20 @@ export abstract class TestNode extends vscode.TreeItem {
    if (outcome === 'Pass') {


Asking because you're in here anyway; is it worth extrapolating all the outcome strings into variables?

good idea..on it

good catch Co-authored-by: Ken Lewis <46458081+klewis-sfdc@users.noreply.github.com>

…rcedotcom/salesforcedx-vscode into gbockus/W-12545057-test-icons-missing

randi274 · 2023-02-17T15:49:31Z

packages/salesforcedx-vscode-apex/src/constants.ts

 export const LSP_ERR = 'apexLSPError';
+
+export const PASS_RESULT = 'Pass';


randi274 · 2023-02-17T15:56:21Z

packages/salesforcedx-vscode-apex/src/views/testOutlineProvider.ts

@@ -34,6 +31,9 @@ const NO_TESTS_DESCRIPTION = nls.localize(
  'force_test_view_no_tests_description'
 );

+const TEST_RUN_ID_FILE = 'test-run-id.txt';
+const TEST_RESULT_JSON_FILE = `test-result.json`;


Nit: ` vs ' here?

randi274 · 2023-02-17T16:02:59Z

QA:
✅ Colorful test indicators appear in both light and dark mode for Apex & LWC tests (confirmed blue, green, red, and orange)
✅ Checkpoints are now added/removed in a logical order, including when a breakpoint is converted
✅ Uploaded checkpoints and used replay debugger to make sure they were properly set
✅ Tested on both Mac and Windows

gbockus-sf · 2023-02-18T00:39:05Z

...dx-vscode-apex-replay-debugger/test/vscode-integration/checkpoints/checkpointService.test.ts

@@ -322,188 +322,6 @@ describe('Verify checkpoint callback for vscode.debug.onDidChangeBreakpoints', (
    expect(lockSpy.getCall(1).args[0]).to.equal(CHECKPOINTS_LOCK_STRING);
  });



These three tests depended on all checkpoints sharing the same _id property (undefined). I tried to see if I could use the real id property to get these tests to still work, but not luck (id is readonly). I also attempted to fake it but b/c there is an instanceof check in the code flow there was no getting around it. Elected to just remove these int tests vs continuing the fight or changing logic to accommodate the int tests.

gbockus-sf added 2 commits February 15, 2023 17:46

fix: update to allow icons to show up in vix

1c6e291

refactor: pull updateDb out of languageServer

516c33f

gbockus-sf requested a review from a team as a code owner February 16, 2023 06:49

gbockus-sf requested a review from randi274 February 16, 2023 06:49

gbockus-sf commented Feb 16, 2023

View reviewed changes

klewis-sfdc reviewed Feb 16, 2023

View reviewed changes

packages/salesforcedx-utils-vscode/test/jest/helpers/extensionUris.test.ts Outdated Show resolved Hide resolved

randi274 reviewed Feb 16, 2023

View reviewed changes

gbockus-sf and others added 3 commits February 17, 2023 01:02

chore: update extensionUris.test.ts

0135ba4

good catch Co-authored-by: Ken Lewis <46458081+klewis-sfdc@users.noreply.github.com>

chore: changes from review

9737e17

Merge branch 'gbockus/W-12545057-test-icons-missing' of github.com:fo…

ad6ae26

…rcedotcom/salesforcedx-vscode into gbockus/W-12545057-test-icons-missing

randi274 reviewed Feb 17, 2023

View reviewed changes

randi274 approved these changes Feb 17, 2023

View reviewed changes

gbockus-sf and others added 3 commits February 17, 2023 16:37

Merge branch 'develop' into gbockus/W-12545057-test-icons-missing

ddd8f80

chore: remove invalid checkpoint int tests

09b14a7

chore: changes from review 2

453aa23

gbockus-sf commented Feb 18, 2023

View reviewed changes

gbockus-sf merged commit 3e50b48 into develop Feb 18, 2023

gbockus-sf deleted the gbockus/W-12545057-test-icons-missing branch February 18, 2023 04:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update test icons to account for bundling, fix apexDB location, and fix checkpoint issue #4686

fix: update test icons to account for bundling, fix apexDB location, and fix checkpoint issue #4686

gbockus-sf commented Feb 16, 2023

gbockus-sf Feb 16, 2023

klewis-sfdc Feb 16, 2023

gbockus-sf Feb 16, 2023

gbockus-sf Feb 16, 2023

gbockus-sf Feb 16, 2023

gbockus-sf Feb 16, 2023

gbockus-sf Feb 16, 2023

gbockus-sf Feb 16, 2023

gbockus-sf Feb 16, 2023

gbockus-sf Feb 16, 2023

klewis-sfdc Feb 16, 2023

gbockus-sf Feb 16, 2023

gbockus-sf Feb 16, 2023

gbockus-sf Feb 16, 2023

gbockus-sf Feb 16, 2023

klewis-sfdc Feb 16, 2023

gbockus-sf Feb 16, 2023

klewis-sfdc Feb 16, 2023

gbockus-sf Feb 16, 2023

gbockus-sf Feb 16, 2023

klewis-sfdc commented Feb 16, 2023

randi274 Feb 16, 2023

gbockus-sf Feb 17, 2023

randi274 Feb 17, 2023

randi274 Feb 17, 2023

randi274 commented Feb 17, 2023

gbockus-sf Feb 18, 2023

		@@ -280,20 +271,20 @@ export abstract class TestNode extends vscode.TreeItem {
		if (outcome === 'Pass') {

		export const LSP_ERR = 'apexLSPError';

		export const PASS_RESULT = 'Pass';

		@@ -322,188 +322,6 @@ describe('Verify checkpoint callback for vscode.debug.onDidChangeBreakpoints', (
		expect(lockSpy.getCall(1).args[0]).to.equal(CHECKPOINTS_LOCK_STRING);
		});

fix: update test icons to account for bundling, fix apexDB location, and fix checkpoint issue #4686

fix: update test icons to account for bundling, fix apexDB location, and fix checkpoint issue #4686

Conversation

gbockus-sf commented Feb 16, 2023

What does this PR do?

What issues does this PR fix or reference?

Functionality Before

Functionality After

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klewis-sfdc commented Feb 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

randi274 commented Feb 17, 2023

Choose a reason for hiding this comment