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

Expose data/helpers to check for no changes in bundle sizes to reduce noise #4153

Merged
merged 2 commits into from
Oct 29, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
31 changes: 16 additions & 15 deletions tools/bundle-size-tools/src/ADO/AdoSizeComparator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { WebApi } from 'azure-devops-node-api';
import JSZip from 'jszip';
import { join } from 'path';
import { BundleComparison, BundleComparisonResult } from '../BundleBuddyTypes';
import { getBaselineCommit, getBuilds, getPriorCommit } from '../utilities';
import { BuildStatus, BuildResult } from 'azure-devops-node-api/interfaces/BuildInterfaces';
import { IADOConstants } from './Constants';
Expand Down Expand Up @@ -75,9 +76,10 @@ export class ADOSizeComparator {
* Create a size comparison message that can be posted to a PR
* @param tagWaiting - If the build should be tagged to be updated when the baseline
* build completes (if it wasn't already complete when the comparison runs)
* @returns The size comparison message
* @returns The size comparison result with formatted message and raw data. In case
* of failure, the message contains the error message and the raw data will be undefined.
*/
public async createSizeComparisonMessage(tagWaiting: boolean): Promise<string> {
public async createSizeComparisonMessage(tagWaiting: boolean): Promise<BundleComparisonResult> {
let baselineCommit: string | undefined = getBaselineCommit();
console.log(`The baseline commit for this PR is ${baselineCommit}`);

Expand All @@ -103,7 +105,7 @@ export class ADOSizeComparator {
if (baselineBuild.id === undefined) {
const message = `Baseline build does not have a build id`;
console.log(message);
return message;
return { message, comparison: undefined };
}

// Baseline build is pending
Expand All @@ -115,7 +117,7 @@ export class ADOSizeComparator {
this.tagBuildAsWaiting(baselineCommit);
}

return message;
return { message, comparison: undefined };
}

// Baseline build failed
Expand All @@ -125,7 +127,7 @@ export class ADOSizeComparator {
baselineCommit
);
console.log(message);
return message;
return { message, comparison: undefined };
}

// Baseline build succeeded
Expand Down Expand Up @@ -153,12 +155,16 @@ export class ADOSizeComparator {
if (baselineCommit === undefined || baselineZip === undefined) {
const message = `Could not find a usable baseline build with search starting at CI ${getBaselineCommit()}`;
console.log(message);
return message;
return { message, comparison: undefined };
}

const message = await this.createMessageFromZip(baselineCommit, baselineZip);
const comparison: BundleComparison[] = await this.createComparisonFromZip(baselineCommit, baselineZip);
console.log(JSON.stringify(comparison));

const message = getCommentForBundleDiff(comparison, baselineCommit);
console.log(message);
return message;

return { message, comparison };
}

private async tagBuildAsWaiting(baselineCommit: string): Promise<void> {
Expand All @@ -173,7 +179,7 @@ export class ADOSizeComparator {
}
}

private async createMessageFromZip(baselineCommit: string, baselineZip: JSZip): Promise<string> {
private async createComparisonFromZip(baselineCommit: string, baselineZip: JSZip): Promise<BundleComparison[]> {
const baselineZipBundlePaths = getBundlePathsFromZipObject(baselineZip);

const prBundleFileSystemPaths = await getBundlePathsFromFileSystem(this.localReportPath);
Expand All @@ -198,11 +204,6 @@ export class ADOSizeComparator {
statsProcessors: DefaultStatsProcessors
});

const bundleComparisons = compareBundles(baselineSummaries, prSummaries);

console.log(JSON.stringify(bundleComparisons));

const message = getCommentForBundleDiff(bundleComparisons, baselineCommit);
return message;
return compareBundles(baselineSummaries, prSummaries);
}
}
9 changes: 9 additions & 0 deletions tools/bundle-size-tools/src/BundleBuddyTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ export interface BundleComparison {
commonBundleMetrics: { [key: string]: { baseline: BundleMetric; compare: BundleMetric } };
}

/**
* The formatted message string of a bundle comparison along with the
* comparison data itself
*/
export type BundleComparisonResult = {
message: string,
comparison: BundleComparison[] | undefined,
};

/**
* Functions used to process a webpack stats file and produce a set of metrics. Some processors may choose
* to work off a bundle specific config file. Note that these config files are optional, so not all bundles
Expand Down
19 changes: 19 additions & 0 deletions tools/bundle-size-tools/src/compareBundles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,22 @@ export function compareBundles(baseline: BundleSummaries, compare: BundleSummari

return results;
}

/**
* Checks if a bundle comparison contains no size changes
* @param comparisons
*/
export function bundlesContainNoChanges(comparisons: BundleComparison[]): boolean {
for (let i = 0; i < comparisons.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

non-functional

Thoughts?
for (const { commonBundleMetrics } of comparisons) {

And below:

const metrics = Object.values(commonBundleMetrics); // values ignores keys
for (const { baseline, compare } of metrics) {

Or an unreadable(?) one-liner:

comparisons.every(({ commonBundleMetrics }) => Object.values(commonBundleMetrics)
    .every(({ baseline, compare }) => baseline.parsedSize === compare.parsedSize));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i like the unreadable one but the other one is good too

const { commonBundleMetrics } = comparisons[i];
let metrics = Object.entries(commonBundleMetrics);
for (let j = 0; j < metrics.length; j++) {
const [ , { baseline, compare }] = metrics[j];
if (baseline.parsedSize !== compare.parsedSize) {
return false;
}
}
}

return true;
}