Skip to content
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

update usage of '.sfdx' dir in isvdebugging #4521

Merged
merged 13 commits into from
Nov 4, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
8 changes: 7 additions & 1 deletion packages/salesforcedx-utils-vscode/src/helpers/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ function relativeStateFolder(): string {
return Global.STATE_FOLDER;
}

function relativeToolsFolder(): string {
const relativePathToToolsFolder = path.join(projectPaths.relativeStateFolder(), TOOLS);
return relativePathToToolsFolder;
}

export const projectPaths = {
stateFolder,
metadataFolder,
Expand All @@ -156,5 +161,6 @@ export const projectPaths = {
sfdxProjectConfig,
toolsFolder,
lwcTestResultsFolder,
relativeStateFolder
relativeStateFolder,
relativeToolsFolder
};
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ jest.mock('@salesforce/core', () => {
describe('test project paths', () => {
const FAKE_WORKSPACE = '/here/is/a/fake/path/to/';
const FAKE_STATE_FOLDER = '.sfdx';
const FAKE_TOOLS_FOLDER = '.sfdx/tools';

describe('test stateFolder', () => {
let getRootWorkspacePathStub: jest.SpyInstance;
Expand Down Expand Up @@ -70,4 +71,15 @@ describe('test project paths', () => {
expect(relativeStateFolder).toEqual(FAKE_STATE_FOLDER);
});
});

describe('test relativeToolsFolder', () => {
it('should be defined', () => {
expect(projectPaths.relativeToolsFolder).toBeDefined();
});

it('should return a path to the relative tools folder', () => {
const relativeToolsFolder = projectPaths.relativeToolsFolder();
expect(relativeToolsFolder).toEqual(FAKE_TOOLS_FOLDER);
Copy link
Contributor

Choose a reason for hiding this comment

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

So for this test you should be verifying that the value returned is the combo of the relative state folder and the tools constant. I'd probably be ok with something like

expect(relativeToolsFolder).toEqual(
        path.join(FAKE_STATE_FOLDER, TOOLS) // TOOLS exported from paths.ts
      );

But even better would be to mock the projectPaths.relativeStateFolder() response for this test and have something like

     relativeStateFolderMock.resolves(FAKE_STATE_FOLDER);
     const relativeToolsFolder = projectPaths.relativeToolsFolder();
      expect(relativeToolsFolder).toEqual(
        path.join(FAKE_STATE_FOLDER, TOOLS) // TOOLS exported from paths.ts
      );

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like your test is failing with

    Expected: ".sfdx/tools"     Received: ".sfdx\\tools"```

another good reason to update to the above suggested method for the assertion.

});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
CommandOutput,
ContinueResponse,
ParametersGatherer,
projectPaths,
SfdxCommandBuilder
} from '@salesforce/salesforcedx-utils-vscode';
import { SpawnOptions } from 'child_process';
Expand Down Expand Up @@ -55,17 +56,15 @@ export interface InstalledPackageInfo {

export class IsvDebugBootstrapExecutor extends SfdxCommandletExecutor<{}> {
public readonly relativeMetdataTempPath = path.join(
'.sfdx',
'tools',
projectPaths.relativeToolsFolder(),
'isvdebuggermdapitmp'
);
public readonly relativeApexPackageXmlPath = path.join(
this.relativeMetdataTempPath,
'package.xml'
);
public readonly relativeInstalledPackagesPath = path.join(
'.sfdx',
'tools',
projectPaths.relativeToolsFolder(),
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize adding a full blown jest unit test suite for this file is probably more lift then is worthwhile for the work being addressed, but you would add a test(s) to verify the relativeInstalledPackagesPath and relativeMetdataTempPath readonly values are set appropriately.

'installed-packages'
);

Expand Down