Skip to content

Commit

Permalink
Merge pull request #45 from cqse/ts/33126_fix_flicker
Browse files Browse the repository at this point in the history
TS-33126 Fix exclude pattern not applying
  • Loading branch information
stahlbauer committed Apr 12, 2023
2 parents 6f49d64 + a8ab48d commit 1cc3f41
Show file tree
Hide file tree
Showing 5 changed files with 200 additions and 134 deletions.
1 change: 1 addition & 0 deletions packages/teamscale-javascript-instrumenter/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ We use [Semantic Versioning](https://semver.org/).
# New Release

- [fix] The instrumented app did send some garbage information not adding value.
- [fix] `--exclude-bundle` failed to exclude file paths staring with `./`

# 0.0.1-beta.51

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
CollectorSpecifier, FileExcludePattern,
CollectorSpecifier,
FileExcludePattern,
InstrumentationTask,
OriginSourcePattern,
SourceMapFileReference,
Expand Down Expand Up @@ -88,6 +89,7 @@ export class IstanbulInstrumenter implements IInstrumenter {
*
* @param collector - The collector to send the coverage information to.
* @param taskElement - The task element to perform the instrumentation for.
* @param excludeBundles - A exclude pattern to restrict which bundles should be instrumented
* @param sourcePattern - A pattern to restrict the instrumentation to only a fraction of the task element.
* @param dumpOriginsFile - A file path where all origins from the source map should be dumped in json format, or undefined if no origins should be dumped
*/
Expand All @@ -98,29 +100,32 @@ export class IstanbulInstrumenter implements IInstrumenter {
sourcePattern: OriginSourcePattern,
dumpOriginsFile: string | undefined
): Promise<TaskResult> {
const inputFileSource = fs.readFileSync(taskElement.fromFile, 'utf8');

// We skip files that we have already instrumented
if (inputFileSource.startsWith(IS_INSTRUMENTED_TOKEN)) {
if (!taskElement.isInPlace()) {
writeToFile(taskElement.toFile, inputFileSource);
}
return new TaskResult(0, 0, 0, 1, 0, 0, 0);
}

// Not all file types are supported by the instrumenter
if (!this.isFileTypeSupported(taskElement.fromFile)) {
if (!taskElement.isInPlace()) {
copyToFile(taskElement.toFile, taskElement.fromFile);
}
return new TaskResult(0, 0, 0, 0, 1, 0, 0);
}

// We might want to skip the instrumentation of the file
if (excludeBundles.isExcluded(taskElement.fromFile)) {
if (!taskElement.isInPlace()) {
writeToFile(taskElement.toFile, inputFileSource);
copyToFile(taskElement.toFile, taskElement.fromFile);
}
return new TaskResult(0, 1, 0, 0, 0, 0, 0);
}

const inputFileSource = fs.readFileSync(taskElement.fromFile, 'utf8');

// We skip files that we have already instrumented
if (inputFileSource.startsWith(IS_INSTRUMENTED_TOKEN)) {
if (!taskElement.isInPlace()) {
writeToFile(taskElement.toFile, inputFileSource);
}
return new TaskResult(0, 0, 0, 1, 0, 0, 0);
}

// Report progress
this.logger.info(`Instrumenting "${path.basename(taskElement.fromFile)}"`);

Expand Down Expand Up @@ -418,3 +423,8 @@ function writeToFile(filePath: string, fileContent: string) {
mkdirp.sync(path.dirname(filePath));
fs.writeFileSync(filePath, fileContent);
}

function copyToFile(targetFilePath: string, sourceFilePath: string) {
mkdirp.sync(path.dirname(targetFilePath));
fs.copyFileSync(sourceFilePath, targetFilePath);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Optional } from 'typescript-optional';
import { Contract } from '@cqse/commons';
import matching from 'micromatch';
import micromatch from 'micromatch';
import path from 'path';

/**
Expand Down Expand Up @@ -100,27 +100,24 @@ export class OriginSourcePattern {

const normalizedOriginFiles = originFiles.map(normalizePath);
if (this.exclude) {
const matchedToExclude = matching(normalizedOriginFiles, this.exclude);
const matchedToExclude = micromatch(normalizedOriginFiles, this.exclude);
if (originFiles.length === matchedToExclude.length) {
return false;
}
}

if (this.include) {
const matchedToInclude = matching(normalizedOriginFiles, this.include || ['**']);
return matchedToInclude.length > 0;
return micromatch.some(normalizedOriginFiles, this.include || ['**']);
}

return true;
}

}

/**
* Pattern describing files (bundles) to not instrument.
*/
export class FileExcludePattern {

/**
* Glob pattern describing a set of files to be excluded in the instrumentation process.
*/
Expand All @@ -134,7 +131,7 @@ export class FileExcludePattern {
* Return `true` if the given `filePath` is matched by any of the patterns in `exclude`.
*/
public isExcluded(filePath: string): boolean {
return matching([filePath], this.exclude).length === 1;
return micromatch.isMatch(normalizePath(filePath), this.exclude);
}
}

Expand Down Expand Up @@ -169,10 +166,7 @@ function normalizePath(toNormalize: string): string {
}

function removeTrailingCurrentWorkingDir(removeFrom: string): string {
return removePrefix(
'webpack:///',
removePrefix('.' + path.sep, removeFrom)
);
return removePrefix('webpack:///', removePrefix('.' + path.sep, removeFrom));
}

function removePrefix(prefix: string, removeFrom: string): string {
Expand Down
167 changes: 95 additions & 72 deletions packages/teamscale-javascript-instrumenter/test/unit/Task.test.ts
Original file line number Diff line number Diff line change
@@ -1,82 +1,105 @@
import { OriginSourcePattern } from '../../src/instrumenter/Task';
import { FileExcludePattern, OriginSourcePattern } from '../../src/instrumenter/Task';

test('Include pattern without extension', () => {
const pattern = new OriginSourcePattern(['./src/app/**/*'], undefined);
const result = pattern.isAnyIncluded(['./src/app/app.components.ts', './src/app/messages/messages.component.ts']);
expect(result).toBeTruthy();
});
describe('OriginSourcePattern', () => {
test('Include pattern without extension', () => {
const pattern = new OriginSourcePattern(['./src/app/**/*'], undefined);
const result = pattern.isAnyIncluded([
'./src/app/app.components.ts',
'./src/app/messages/messages.component.ts'
]);
expect(result).toBeTruthy();
});

test('Include pattern with extension', () => {
const pattern = new OriginSourcePattern(['src/app/**/*.ts'], undefined);
const result = pattern.isAnyIncluded(['src/app/app.components.ts', 'src/app/messages/messages.component.ts']);
expect(result).toBeTruthy();
});
test('Include pattern with extension', () => {
const pattern = new OriginSourcePattern(['src/app/**/*.ts'], undefined);
const result = pattern.isAnyIncluded(['src/app/app.components.ts', 'src/app/messages/messages.component.ts']);
expect(result).toBeTruthy();
});

test('Include pattern with extension. Path normalization', () => {
const pattern = new OriginSourcePattern(['src/app/**/*.ts'], undefined);
const result = pattern.isAnyIncluded(['./src/app/app.components.ts', './src/app/messages/messages.component.ts']);
expect(result).toBeTruthy();
});
test('Include pattern with extension. Path normalization', () => {
const pattern = new OriginSourcePattern(['src/app/**/*.ts'], undefined);
const result = pattern.isAnyIncluded([
'./src/app/app.components.ts',
'./src/app/messages/messages.component.ts'
]);
expect(result).toBeTruthy();
});

test('Exclude pattern with extension. Path normalization', () => {
const pattern = new OriginSourcePattern(undefined, ['src/app/**/*.ts']);
expect(
pattern.isAnyIncluded(['./berg/app/app.components.ts', './src/app/messages/messages.component.ts'])
).toBeTruthy();
expect(pattern.isAnyIncluded(['./berg/app/app.components.ts'])).toBeTruthy();
expect(pattern.isAnyIncluded(['./src/app/app.components.ts'])).toBeFalsy();
});
test('Exclude pattern with extension. Path normalization', () => {
const pattern = new OriginSourcePattern(undefined, ['src/app/**/*.ts']);
expect(
pattern.isAnyIncluded(['./berg/app/app.components.ts', './src/app/messages/messages.component.ts'])
).toBeTruthy();
expect(pattern.isAnyIncluded(['./berg/app/app.components.ts'])).toBeTruthy();
expect(pattern.isAnyIncluded(['./src/app/app.components.ts'])).toBeFalsy();
});

test('Exclude and include pattern', () => {
const pattern = new OriginSourcePattern(['src/bar/**/*.ts'], ['src/foo/**/*.ts']);
expect(
pattern.isAnyIncluded(['./src/bar/app.components.ts', './src/foo/messages/messages.component.ts'])
).toBeTruthy();
expect(pattern.isAnyIncluded(['./src/foo/messages/messages.component.ts'])).toBeFalsy();
expect(pattern.isAnyIncluded(['./src/bar/messages/messages.component.ts'])).toBeTruthy();
expect(
pattern.isAnyIncluded([
'./test/foo/unittest.test.ts',
'./test/bar/unittest.test.ts',
'./build/main.ts',
'main.ts'
])
).toBeFalsy();
});
test('Exclude and include pattern', () => {
const pattern = new OriginSourcePattern(['src/bar/**/*.ts'], ['src/foo/**/*.ts']);
expect(
pattern.isAnyIncluded(['./src/bar/app.components.ts', './src/foo/messages/messages.component.ts'])
).toBeTruthy();
expect(pattern.isAnyIncluded(['./src/foo/messages/messages.component.ts'])).toBeFalsy();
expect(pattern.isAnyIncluded(['./src/bar/messages/messages.component.ts'])).toBeTruthy();
expect(
pattern.isAnyIncluded([
'./test/foo/unittest.test.ts',
'./test/bar/unittest.test.ts',
'./build/main.ts',
'main.ts'
])
).toBeFalsy();
});

test('Exclude patterns only', () => {
const pattern = new OriginSourcePattern([], ['src/foo/messages/*.ts']);
expect(
pattern.isAnyIncluded(['./src/bar/app.components.ts', './src/foo/messages/messages.component.ts'])
).toBeTruthy();
expect(pattern.isAnyIncluded(['./src/foo/messages/messages.component.ts'])).toBeFalsy();
expect(pattern.isAnyIncluded(['src/foo/messages/messages.component.ts'])).toBeFalsy();
expect(pattern.isAnyIncluded(['webpack:///src/foo/messages/messages.component.ts'])).toBeFalsy();
expect(pattern.isAnyIncluded(['./src/bar/messages/messages.component.ts'])).toBeTruthy();
expect(pattern.isAnyIncluded(['./test/bar/unittest.test.ts', './build/main.ts', 'main.ts'])).toBeTruthy();
});
test('Exclude patterns only', () => {
const pattern = new OriginSourcePattern([], ['src/foo/messages/*.ts']);
expect(
pattern.isAnyIncluded(['./src/bar/app.components.ts', './src/foo/messages/messages.component.ts'])
).toBeTruthy();
expect(pattern.isAnyIncluded(['./src/foo/messages/messages.component.ts'])).toBeFalsy();
expect(pattern.isAnyIncluded(['src/foo/messages/messages.component.ts'])).toBeFalsy();
expect(pattern.isAnyIncluded(['webpack:///src/foo/messages/messages.component.ts'])).toBeFalsy();
expect(pattern.isAnyIncluded(['./src/bar/messages/messages.component.ts'])).toBeTruthy();
expect(pattern.isAnyIncluded(['./test/bar/unittest.test.ts', './build/main.ts', 'main.ts'])).toBeTruthy();
});

test('Exclude and include pattern on file extensions', () => {
const pattern = new OriginSourcePattern(
['**/*.java', '**/*.md'],
['**/*.cc', '**/*.cpp', '**/*.h', '**/*.hpp']
);
expect(pattern.isAnyIncluded(['./ServerConnector.java', './Server.h'])).toBeTruthy();
expect(
pattern.isAnyIncluded(['./ServerConnector.java', './ServerVerifier.java', './ServerStarter.java'])
).toBeTruthy();
expect(
pattern.isAnyIncluded([
'./ServerConnector.java',
'./ServerVerifier.java',
'./ServerStarter.java',
'main.cpp',
'Logger.java'
])
).toBeTruthy();
expect(pattern.isAnyIncluded(['proj/docs/README.md'])).toBeTruthy();
expect(pattern.isAnyIncluded(['proj/src/reader.cpp', 'proj/test/reader_test.cpp'])).toBeFalsy();
});

test('Exclude and include pattern on file extensions', () => {
const pattern = new OriginSourcePattern(['**/*.java', '**/*.md'], ['**/*.cc', '**/*.cpp', '**/*.h', '**/*.hpp']);
expect(pattern.isAnyIncluded(['./ServerConnector.java', './Server.h'])).toBeTruthy();
expect(
pattern.isAnyIncluded(['./ServerConnector.java', './ServerVerifier.java', './ServerStarter.java'])
).toBeTruthy();
expect(
pattern.isAnyIncluded([
'./ServerConnector.java',
'./ServerVerifier.java',
'./ServerStarter.java',
'main.cpp',
'Logger.java'
])
).toBeTruthy();
expect(pattern.isAnyIncluded(['proj/docs/README.md'])).toBeTruthy();
expect(pattern.isAnyIncluded(['proj/src/reader.cpp', 'proj/test/reader_test.cpp'])).toBeFalsy();
test('Exclude and include pattern precedence', () => {
const pattern = new OriginSourcePattern(
['**/ab/**', '**/cd/**'],
['**/ef/**', '**/gh/**', '**/ij/**', '**/kl/**']
);
expect(pattern.isAnyIncluded(['./xy/ef/file1.ts', './kl/file2.ts'])).toBeFalsy();
expect(pattern.isAnyIncluded(['./xy/ef/file1.ts', './kl/file2.ts', './xy/ij/ab/file3.ts'])).toBeFalsy(); // exclude has precedence over include
});
});

test('Exclude and include pattern precedence', () => {
const pattern = new OriginSourcePattern(['**/ab/**', '**/cd/**'], ['**/ef/**', '**/gh/**', '**/ij/**', '**/kl/**']);
expect(pattern.isAnyIncluded(['./xy/ef/file1.ts', './kl/file2.ts'])).toBeFalsy();
expect(pattern.isAnyIncluded(['./xy/ef/file1.ts', './kl/file2.ts', './xy/ij/ab/file3.ts'])).toBeFalsy(); // exclude has precedence over include
describe('FileExcludePattern', () => {
test('Exclude pattern with extension. Path normalization', () => {
const pattern = new FileExcludePattern(['./**/(runtime|vendor)*.js']);
expect(pattern.isExcluded('./src/app/index.js')).toBeFalsy();
expect(pattern.isExcluded('vendor.5d3bc21.js')).toBeTruthy();
expect(pattern.isExcluded('./assets/vendor.5d3bc21.js')).toBeTruthy();
});
});

0 comments on commit 1cc3f41

Please sign in to comment.