Skip to content

Commit

Permalink
refactor: shared posixify fn for windows ut
Browse files Browse the repository at this point in the history
  • Loading branch information
@mshanemc
mshanemc committed Aug 1, 2024
1 parent fffaea9 commit 42eb6d7
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 27 deletions.
4 changes: 3 additions & 1 deletion src/client/deployMessages.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { basename, dirname, extname, join, posix, sep } from 'node:path/posix';
import { SfError } from '@salesforce/core';
import { ensureArray } from '@salesforce/kit';
import { posixify } from '../utils/path';
import { ComponentLike, SourceComponent } from '../resolve';
import { registry } from '../registry/registry';
import {
Expand Down Expand Up @@ -154,8 +155,9 @@ const hasComponentType = (message: DeployMessage): message is DeployMessage & {

export const toKey = (component: ComponentLike): string => {
const type = typeof component.type === 'string' ? component.type : component.type.name;
return `${type}#${shouldConvertPaths ? component.fullName.split(sep).join(posix.sep) : component.fullName}`;
return `${type}#${shouldConvertPaths ? posixify(component.fullName) : component.fullName}`;
};

const isTrue = (value: BooleanString): boolean => value === 'true' || value === true;

export const shouldConvertPaths = sep !== posix.sep;
4 changes: 2 additions & 2 deletions src/client/metadataApiDeploy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import JSZip from 'jszip';
import fs from 'graceful-fs';
import { Lifecycle, Messages, SfError } from '@salesforce/core';
import { ensureArray } from '@salesforce/kit';
import { posixify } from '../utils/path';
import { RegistryAccess } from '../registry/registryAccess';
import { ReplacementEvent } from '../convert/types';
import { MetadataConverter } from '../convert/metadataConverter';
Expand Down Expand Up @@ -311,8 +312,7 @@ export class MetadataApiDeploy extends MetadataTransfer<
// Add relative file paths to a root of "zip" for MDAPI.
const relPath = join('zip', relative(mdapiPath, fullPath));
// Ensure only posix paths are added to zip files
const relPosixPath = relPath.replace(/\\/g, '/');
zip.file(relPosixPath, fs.createReadStream(fullPath));
zip.file(posixify(relPath), fs.createReadStream(fullPath));
}
}
};
Expand Down
7 changes: 3 additions & 4 deletions src/convert/replacements.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
*/
import { readFile } from 'node:fs/promises';
import { Transform, Readable } from 'node:stream';
import { sep, posix, join, isAbsolute } from 'node:path';
import { join, isAbsolute } from 'node:path';
import { Lifecycle, Messages, SfError, SfProject } from '@salesforce/core';
import { minimatch } from 'minimatch';
import { Env } from '@salesforce/kit';
import { ensureString, isString } from '@salesforce/ts-types';
import { posixify } from '../utils/path';
import { SourcePath } from '../common/types';
import { SourceComponent } from '../resolve/sourceComponent';
import { MarkedReplacement, ReplacementConfig, ReplacementEvent } from './types';
Expand Down Expand Up @@ -199,7 +200,7 @@ export const matchesFile =
(r: ReplacementConfig): boolean =>
// filenames will be absolute. We don't have convenient access to the pkgDirs,
// so we need to be more open than an exact match
(typeof r.filename === 'string' && posixifyPaths(filename).endsWith(r.filename)) ||
(typeof r.filename === 'string' && posixify(filename).endsWith(r.filename)) ||
(typeof r.glob === 'string' && minimatch(filename, `**/${r.glob}`));

/**
Expand Down Expand Up @@ -245,8 +246,6 @@ export const stringToRegex = (input: string): RegExp =>
// eslint-disable-next-line no-useless-escape
new RegExp(input.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&'), 'g');

export const posixifyPaths = (f: string): string => f.split(sep).join(posix.sep);

/** if replaceWithFile is present, resolve it to an absolute path relative to the projectdir */
const makeAbsolute =
(projectDir: string) =>
Expand Down
4 changes: 2 additions & 2 deletions src/convert/streams.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { createWriteStream, existsSync, promises as fsPromises } from 'graceful-
import { JsonMap } from '@salesforce/ts-types';
import { XMLBuilder } from 'fast-xml-parser';
import { Logger } from '@salesforce/core';
import { posixify } from '../utils/path';
import { SourceComponent } from '../resolve/sourceComponent';
import { SourcePath } from '../common/types';
import { XML_COMMENT_PROP_NAME, XML_DECL } from '../common/constants';
Expand Down Expand Up @@ -234,8 +235,7 @@ export class ZipWriter extends ComponentWriter {

public addToZip(contents: string | Readable | Buffer, path: SourcePath): void {
// Ensure only posix paths are added to zip files
const posixPath = path.replace(/\\/g, '/');
this.zip.file(posixPath, contents);
this.zip.file(posixify(path), contents);
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/convert/transformers/staticResourceMetadataTransformer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { JsonMap } from '@salesforce/ts-types';
import { createWriteStream } from 'graceful-fs';
import { Logger, Messages, SfError } from '@salesforce/core';
import { isEmpty } from '@salesforce/kit';
import { baseName } from '../../utils/path';
import { baseName, posixify } from '../../utils/path';
import { WriteInfo } from '../types';
import { SourceComponent } from '../../resolve/sourceComponent';
import { SourcePath } from '../../common/types';
Expand Down Expand Up @@ -59,8 +59,7 @@ export class StaticResourceMetadataTransformer extends BaseMetadataTransformer {
// have to walk the component content. Replacements only happen if set on the component.
for (const path of component.walkContent()) {
const replacementStream = getReplacementStreamForReadable(component, path);
const relPath = relative(content, path);
const relPosixPath = relPath.replace(/\\/g, '/');
const relPosixPath = posixify(relative(content, path));
zip.file(relPosixPath, replacementStream);
}

Expand Down
3 changes: 3 additions & 0 deletions src/utils/path.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { basename, dirname, extname, sep, join } from 'node:path';
import { posix } from 'node:path/posix';
import { Optional } from '@salesforce/ts-types';
import { SfdxFileFormat } from '../convert/types';
import { SourcePath } from '../common/types';
Expand Down Expand Up @@ -161,3 +162,5 @@ export const fnJoin =
(a: string) =>
(b: string): string =>
join(a, b);

export const posixify = (f: string): string => f.split(sep).join(posix.sep);
20 changes: 10 additions & 10 deletions test/convert/replacements.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import {
matchesFile,
replacementIterations,
stringToRegex,
posixifyPaths,
envFilter,
} from '../../src/convert/replacements';
import { matchingContentFile } from '../mock';
import * as replacementsForMock from '../../src/convert/replacements';
import { posixify } from '../../src/utils/path';

config.truncateThreshold = 0;

Expand Down Expand Up @@ -155,7 +155,7 @@ describe('marking replacements on a component', () => {
assert(cmp.xml);
const result = await getReplacements(cmp, [
// spec says filename path should be posix. The mocks are using join, so on windows they are wrong
{ filename: posixifyPaths(cmp.xml), stringToReplace: 'foo', replaceWithEnv: 'FOO_REPLACEMENT' },
{ filename: posixify(cmp.xml), stringToReplace: 'foo', replaceWithEnv: 'FOO_REPLACEMENT' },
]);
expect(result).to.deep.equal({
[cmp.xml]: [
Expand All @@ -171,7 +171,7 @@ describe('marking replacements on a component', () => {
it('marks string replacements from file', async () => {
assert(cmp.xml);
const result = await getReplacements(cmp, [
{ filename: posixifyPaths(cmp.xml), stringToReplace: 'foo', replaceWithFile: 'bar' },
{ filename: posixify(cmp.xml), stringToReplace: 'foo', replaceWithFile: 'bar' },
]);
expect(result).to.deep.equal({
[cmp.xml]: [
Expand All @@ -188,7 +188,7 @@ describe('marking replacements on a component', () => {
it('marks regex replacements on a matching file', async () => {
assert(cmp.xml);
const result = await getReplacements(cmp, [
{ filename: posixifyPaths(cmp.xml), regexToReplace: '.*foo.*', replaceWithEnv: 'FOO_REPLACEMENT' },
{ filename: posixify(cmp.xml), regexToReplace: '.*foo.*', replaceWithEnv: 'FOO_REPLACEMENT' },
]);
expect(result).to.deep.equal({
[cmp.xml]: [
Expand All @@ -204,8 +204,8 @@ describe('marking replacements on a component', () => {
it('marks 2 replacements on one file', async () => {
assert(cmp.xml);
const result = await getReplacements(cmp, [
{ filename: posixifyPaths(cmp.xml), stringToReplace: 'foo', replaceWithEnv: 'FOO_REPLACEMENT' },
{ filename: posixifyPaths(cmp.xml), stringToReplace: 'baz', replaceWithEnv: 'FOO_REPLACEMENT' },
{ filename: posixify(cmp.xml), stringToReplace: 'foo', replaceWithEnv: 'FOO_REPLACEMENT' },
{ filename: posixify(cmp.xml), stringToReplace: 'baz', replaceWithEnv: 'FOO_REPLACEMENT' },
]);
expect(result).to.deep.equal({
[cmp.xml]: [
Expand Down Expand Up @@ -253,8 +253,8 @@ describe('marking replacements on a component', () => {
assert(cmp.content);
assert(cmp.xml);
const result = await getReplacements(cmp, [
{ filename: posixifyPaths(cmp.xml), stringToReplace: 'foo', replaceWithEnv: 'FOO_REPLACEMENT' },
{ filename: posixifyPaths(cmp.content), stringToReplace: 'foo', replaceWithEnv: 'FOO_REPLACEMENT' },
{ filename: posixify(cmp.xml), stringToReplace: 'foo', replaceWithEnv: 'FOO_REPLACEMENT' },
{ filename: posixify(cmp.content), stringToReplace: 'foo', replaceWithEnv: 'FOO_REPLACEMENT' },
]);
expect(result).to.deep.equal({
[cmp.xml]: [
Expand All @@ -280,7 +280,7 @@ describe('marking replacements on a component', () => {
assert(cmp.xml);
try {
await getReplacements(cmp, [
{ filename: posixifyPaths(cmp.xml), regexToReplace: '.*foo.*', replaceWithEnv: 'BAD_ENV' },
{ filename: posixify(cmp.xml), regexToReplace: '.*foo.*', replaceWithEnv: 'BAD_ENV' },
]);
assert.fail('should have thrown');
} catch (e) {
Expand All @@ -291,7 +291,7 @@ describe('marking replacements on a component', () => {
assert(cmp.xml);
const result = await getReplacements(cmp, [
{
filename: posixifyPaths(cmp.xml),
filename: posixify(cmp.xml),
regexToReplace: '.*foo.*',
replaceWithEnv: 'BAD_ENV',
allowUnsetEnvVariable: true,
Expand Down
8 changes: 3 additions & 5 deletions test/mock/client/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import JSZip from 'jszip';
import { posixify } from '../../../src/utils/path';

// eslint-disable-next-line @typescript-eslint/require-await
export async function createMockZip(entries: string[]): Promise<Buffer> {
const zip = JSZip();
for (const entry of entries) {
// Ensure only posix paths are added to zip files
const relPosixPath = entry.replace(/\\/g, '/');
zip.file(relPosixPath, '');
}
// Ensure only posix paths are added to zip files
entries.map((entry) => zip.file(posixify(entry), ''));
return zip.generateAsync({
type: 'nodebuffer',
compression: 'DEFLATE',
Expand Down

2 comments on commit 42eb6d7

@svc-cli-bot
Copy link
Collaborator

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: 42eb6d7 Previous: fffaea9 Ratio
eda-componentSetCreate-linux 183 ms 178 ms 1.03
eda-sourceToMdapi-linux 2186 ms 2323 ms 0.94
eda-sourceToZip-linux 1813 ms 1850 ms 0.98
eda-mdapiToSource-linux 2882 ms 2865 ms 1.01
lotsOfClasses-componentSetCreate-linux 358 ms 364 ms 0.98
lotsOfClasses-sourceToMdapi-linux 3596 ms 3732 ms 0.96
lotsOfClasses-sourceToZip-linux 3141 ms 3098 ms 1.01
lotsOfClasses-mdapiToSource-linux 3487 ms 3560 ms 0.98
lotsOfClassesOneDir-componentSetCreate-linux 616 ms 636 ms 0.97
lotsOfClassesOneDir-sourceToMdapi-linux 6390 ms 6368 ms 1.00
lotsOfClassesOneDir-sourceToZip-linux 5529 ms 5592 ms 0.99
lotsOfClassesOneDir-mdapiToSource-linux 6453 ms 6355 ms 1.02

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Collaborator

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: 42eb6d7 Previous: fffaea9 Ratio
eda-componentSetCreate-win32 390 ms 454 ms 0.86
eda-sourceToMdapi-win32 4015 ms 4536 ms 0.89
eda-sourceToZip-win32 2701 ms 2998 ms 0.90
eda-mdapiToSource-win32 5705 ms 6016 ms 0.95
lotsOfClasses-componentSetCreate-win32 896 ms 921 ms 0.97
lotsOfClasses-sourceToMdapi-win32 7325 ms 7890 ms 0.93
lotsOfClasses-sourceToZip-win32 4721 ms 4954 ms 0.95
lotsOfClasses-mdapiToSource-win32 7613 ms 8026 ms 0.95
lotsOfClassesOneDir-componentSetCreate-win32 1486 ms 1465 ms 1.01
lotsOfClassesOneDir-sourceToMdapi-win32 13205 ms 14323 ms 0.92
lotsOfClassesOneDir-sourceToZip-win32 8399 ms 9038 ms 0.93
lotsOfClassesOneDir-mdapiToSource-win32 13500 ms 13321 ms 1.01

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.