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 10 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
9 changes: 8 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,7 @@ export const projectPaths = {
sfdxProjectConfig,
toolsFolder,
lwcTestResultsFolder,
relativeStateFolder
relativeStateFolder,
relativeToolsFolder,
TOOLS
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I'd just export the TOOLS constant on line 17 vs including constants and functions here.

};
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,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(path.join(FAKE_STATE_FOLDER, projectPaths.TOOLS));
});
});
});
4 changes: 4 additions & 0 deletions packages/salesforcedx-vscode-core/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
//eslint:disable-next-line:no-var-requires
const baseConfig = require('../../config/jest.base.config');

module.exports = Object.assign({}, baseConfig);
3 changes: 2 additions & 1 deletion packages/salesforcedx-vscode-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@
"clean": "shx rm -rf node_modules && shx rm -rf out && shx rm -rf coverage && shx rm -rf .nyc_output",
"test": "npm run test:vscode-integration",
"test:vscode-integration": "cross-env CODE_TESTS_WORKSPACE='../system-tests/assets/sfdx-simple' node ../../scripts/run-vscode-integration-tests",
"test:vscode-insiders-integration": "cross-env CODE_VERSION=insiders npm run test:vscode-integration"
"test:vscode-insiders-integration": "cross-env CODE_VERSION=insiders npm run test:vscode-integration",
"test:unit": "jest --coverage"
},
"nyc": {
"reporter": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
} from '@salesforce/source-deploy-retrieve/lib/src/client/types';
Copy link
Contributor

Choose a reason for hiding this comment

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

Some touches to get jest unit tests working in core.

import { join } from 'path';
import * as vscode from 'vscode';
import { BaseDeployExecutor } from '.';
Copy link
Contributor

Choose a reason for hiding this comment

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

So doing an import like this results in scanning all things exported from the index.ts file. Which can lead to complications in unit testing. Updated to pull directly from the file where exported instead.

import { channelService, OUTPUT_CHANNEL } from '../channels';
import { PersistentStorageService } from '../conflict/persistentStorageService';
import { TELEMETRY_METADATA_COUNT } from '../constants';
Expand All @@ -36,6 +35,7 @@ import { nls } from '../messages';
import { DeployQueue } from '../settings';
import { SfdxPackageDirectories } from '../sfdxProject';
import { OrgAuthInfo } from '../util';
import { BaseDeployExecutor } from './baseDeployCommand';
import { createComponentCount, formatException } from './util';

type DeployRetrieveResult = DeployResult | RetrieveResult;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,12 @@
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import {
Command,
SfdxCommandBuilder
} from '@salesforce/salesforcedx-utils-vscode';
import { ContinueResponse } from '@salesforce/salesforcedx-utils-vscode';
import { ContinueResponse, SfdxCommandBuilder } from '@salesforce/salesforcedx-utils-vscode';
import { ComponentSet } from '@salesforce/source-deploy-retrieve';
import { join } from 'path';
import * as vscode from 'vscode';
import { channelService } from '../channels';
import {
ConflictDetectionMessages,
TimestampConflictChecker
} from '../commands/util/postconditionCheckers';
import { nls } from '../messages';
Expand All @@ -23,7 +18,7 @@ import { SfdxPackageDirectories } from '../sfdxProject';
import { telemetryService } from '../telemetry';
import { workspaceUtils } from '../util';
import { DeployExecutor } from './baseDeployRetrieve';
import { FilePathGatherer, SfdxCommandlet, SfdxWorkspaceChecker } from './util';
import { ConflictDetectionMessages, FilePathGatherer, SfdxCommandlet, SfdxWorkspaceChecker } from './util';
Copy link
Contributor

Choose a reason for hiding this comment

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

Ran an organize imports. quite a few unused here.


export class LibrarySourceDeployManifestExecutor extends DeployExecutor<
string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ import { telemetryService } from '../telemetry';
import { DeployExecutor } from './baseDeployRetrieve';
import { SourcePathChecker } from './forceSourceRetrieveSourcePath';
import {
ConflictDetectionMessages,
LibraryPathsGatherer,
SfdxCommandlet,
SfdxWorkspaceChecker
} from './util';
import {
CompositePostconditionChecker,
ConflictDetectionMessages,
TimestampConflictChecker
} from './util/postconditionCheckers';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ import { SfdxPackageDirectories, SfdxProjectConfig } from '../sfdxProject';
import { telemetryService } from '../telemetry';
import { RetrieveExecutor } from './baseDeployRetrieve';
import {
ConflictDetectionMessages,
LibraryPathsGatherer,
SfdxCommandlet,
SfdxWorkspaceChecker
} from './util';
import { ConflictDetectionMessages } from './util/postconditionCheckers';

export class LibraryRetrieveSourcePathExecutor extends RetrieveExecutor<
string[]
Expand Down
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 @@ -53,19 +54,21 @@ export interface InstalledPackageInfo {
versionNumber: string;
}

export const ISVDEBUGGER = 'isvdebuggermdapitmp';
export const INSTALLED_PACKAGES = 'installed-packages';
export const PACKAGE_XML = 'package.xml';
Copy link
Contributor

Choose a reason for hiding this comment

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

we should update the below paths to use these constants.


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
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright (c) 2022, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

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 these two exports they were originally in postconditionCheckers.ts with a ton of other stuff. This lead to always importing this metadata caching checker that had a ton of dependancies and eventually had issues. Simplified by moving these out to their own files and re-exporting from there for index.ts.

export interface ConflictDetectionMessages {
warningMessageKey: string;
commandHint: (input: string | string[]) => string;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright (c) 2022, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { CancelResponse, ContinueResponse, PostconditionChecker } from '@salesforce/salesforcedx-utils-vscode';

export class EmptyPostChecker implements PostconditionChecker<any> {
public async check(
inputs: ContinueResponse<any> | CancelResponse
): Promise<ContinueResponse<any> | CancelResponse> {
return inputs;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ export {
SelectUsername
} from './parameterGatherers';
export {
ConflictDetectionMessages,
EmptyPostChecker
} from './postconditionCheckers';
ConflictDetectionMessages
} from './conflictDetectionMessages';
export {EmptyPostChecker} from './emptyPostChecker';
export {
CommandletExecutor,
CommandParams,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { notificationService } from '../../notifications';
import { DeployQueue, sfdxCoreSettings } from '../../settings';
import { telemetryService } from '../../telemetry';
import { MetadataDictionary, workspaceUtils } from '../../util';
import { ConflictDetectionMessages } from './conflictDetectionMessages';
import { PathStrategyFactory } from './sourcePathStrategies';

type OneOrMany = LocalComponent | LocalComponent[];
Expand Down Expand Up @@ -54,14 +55,6 @@ export class CompositePostconditionChecker<T>
}
}

export class EmptyPostChecker implements PostconditionChecker<any> {
public async check(
inputs: ContinueResponse<any> | CancelResponse
): Promise<ContinueResponse<any> | CancelResponse> {
return inputs;
}
}

/* tslint:disable-next-line:prefer-for-of */
export class OverwriteComponentPrompt
implements PostconditionChecker<OneOrMany> {
Expand Down Expand Up @@ -206,11 +199,6 @@ export class OverwriteComponentPrompt
}
}

export interface ConflictDetectionMessages {
warningMessageKey: string;
commandHint: (input: string | string[]) => string;
}

export class TimestampConflictChecker implements PostconditionChecker<string> {
private isManifest: boolean;
private messages: ConflictDetectionMessages;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,19 @@
import {
CliCommandExecutor,
Command,
CommandExecution
} from '@salesforce/salesforcedx-utils-vscode';
import {
Measurements,
Properties,
TelemetryData
} from '@salesforce/salesforcedx-utils-vscode';
import {
ContinueResponse,
ParametersGatherer,
CommandExecution, ContinueResponse, Measurements, ParametersGatherer,
PostconditionChecker,
PreconditionChecker
PreconditionChecker, Properties,
TelemetryData
} from '@salesforce/salesforcedx-utils-vscode';
import * as vscode from 'vscode';
import { EmptyPostChecker } from '.';
import { channelService } from '../../channels';
import { notificationService, ProgressNotification } from '../../notifications';
import { sfdxCoreSettings } from '../../settings';
import { taskViewService } from '../../statuses';
import { telemetryService } from '../../telemetry';
import { workspaceUtils } from '../../util';
import { EmptyPostChecker } from './emptyPostChecker';
Copy link
Contributor

Choose a reason for hiding this comment

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

organize imports.


export enum CommandVersion {
Beta = 'beta',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { projectPaths } from '@salesforce/salesforcedx-utils-vscode';
import * as path from 'path';
import { IsvDebugBootstrapExecutor, ISVDEBUGGER } from '../../../../src/commands/isvdebugging/bootstrapCmd';

describe('isvdebugging unit test', () => {

const TOOLS_FOLDER = 'tools';
let relativeToolsFolderStub: jest.SpyInstance;

let isvDebugBootstrapExecutorInst: IsvDebugBootstrapExecutor;

beforeEach(() => {
relativeToolsFolderStub = jest.spyOn(projectPaths, 'relativeToolsFolder').mockReturnValue(TOOLS_FOLDER);
});

it('should be defined', () => {
isvDebugBootstrapExecutorInst = new IsvDebugBootstrapExecutor();
Copy link
Contributor

Choose a reason for hiding this comment

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

So this wasn't required, but when testing an instance of a class it's always good to create the instance for each test.

expect(isvDebugBootstrapExecutorInst).toBeDefined();
});

it('should test readonly relativeMetdataTempPath property', () => {
isvDebugBootstrapExecutorInst = new IsvDebugBootstrapExecutor();
expect(isvDebugBootstrapExecutorInst.relativeMetdataTempPath).toEqual(path.join(TOOLS_FOLDER, ISVDEBUGGER));
});
});
10 changes: 9 additions & 1 deletion scripts/setup-jest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,17 @@ const getMockVSCode = () => {
};
public dispose = () => {};
},
TreeItem: jest.fn(),
commands: jest.fn(),
Disposable: jest.fn(),
env: {
machineId: '12345534'
},
EventEmitter: EventEmitter,
ExtensionMode: { Production: 1, Development: 2, Test: 3 },
languages: {
createDiagnosticCollection: jest.fn()
},
Uri: {
parse: jest.fn(),
file: jest.fn()
Expand Down Expand Up @@ -63,7 +67,11 @@ const getMockVSCode = () => {
};
},
onDidChangeConfiguration: jest.fn(),
createFileSystemWatcher: jest.fn(),
createFileSystemWatcher: jest.fn().mockReturnValue({
onDidChange: jest.fn(),
onDidCreate: jest.fn(),
onDidDelete: jest.fn()
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

new vscode mock setup to get things working for core. Note that b/c the src/context/index.ts creates an instance of WorkspaceContext on initialization
export const workspaceContext = WorkspaceContext.getInstance();
We need to mock the file watcher results prior to loading any test dependancies.
This is an anti-pattern we should avoid for singletons.

workspaceFolders: []
}
};
Expand Down