Skip to content

Commit

Permalink
test: should not call stats toJson if not necessary (#9434)
Browse files Browse the repository at this point in the history 
test: not call stats toJson if not necessary
  • Loading branch information
@LingyuCoder
LingyuCoder authored Feb 24, 2025
1 parent 4b72f57 commit 121557a
Show file tree
Hide file tree
Showing 20 changed files with 186 additions and 67 deletions.
13 changes: 11 additions & 2 deletions packages/rspack-test-tools/etc/test-tools.api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import type EventEmitter from 'node:events';
import { IBasicGlobalContext as IBasicGlobalContext_2 } from '../../type';
import { IBasicModuleScope as IBasicModuleScope_2 } from '../../type';
import { ITestCompilerManager as ITestCompilerManager_2 } from '../type';
import type { MultiStats } from '@rspack/core';
import type { MultiStats as MultiStats_2 } from 'webpack';
import type { RspackOptions } from '@rspack/core';
import type { RspackOptionsNormalized } from '@rspack/core';
import type { RspackPluginInstance } from '@rspack/core';
Expand Down Expand Up @@ -934,6 +936,8 @@ export interface IStatsAPIProcessorOptions<T extends ECompilerType> {
export interface IStatsProcessorOptions<T extends ECompilerType> extends Omit<IMultiTaskProcessorOptions<T>, "runable"> {
// (undocumented)
snapshotName?: string;
// (undocumented)
writeStatsOuptut?: boolean;
}

// @public (undocumented)
Expand All @@ -957,7 +961,7 @@ export interface ITestCompilerManager<T extends ECompilerType> {
// (undocumented)
getOptions(): TCompilerOptions<T>;
// (undocumented)
getStats(): TCompilerStats<T> | null;
getStats(): TCompilerStats<T> | TCompilerMultiStats<T> | null;
// (undocumented)
mergeOptions(newOptions: TCompilerOptions<T>): TCompilerOptions<T>;
// (undocumented)
Expand Down Expand Up @@ -1366,6 +1370,9 @@ export type TCompilerFactories = Record<ECompilerType, TCompilerFactory<ECompile
// @public (undocumented)
export type TCompilerFactory<T extends ECompilerType> = (options: TCompilerOptions<T> | TCompilerOptions<T>[]) => TCompiler<T>;

// @public (undocumented)
export type TCompilerMultiStats<T> = T extends ECompilerType.Rspack ? MultiStats : MultiStats_2;

// @public (undocumented)
export type TCompilerOptions<T> = T extends ECompilerType.Rspack ? RspackOptions : Configuration;

Expand Down Expand Up @@ -1553,8 +1560,10 @@ export type TStatsAPICaseConfig = Omit<IStatsAPIProcessorOptions<ECompilerType.R
// @public (undocumented)
export type TTestConfig<T extends ECompilerType> = {
documentType?: EDocumentType;
validate?: (stats: TCompilerStats<T>, stderr?: string) => void;
validate?: (stats: TCompilerStats<T> | TCompilerMultiStats<T>, stderr?: string) => void;
noTest?: boolean;
writeStatsOuptut?: boolean;
writeStatsJson?: boolean;
beforeExecute?: () => void;
afterExecute?: () => void;
moduleScope?: (ms: IBasicModuleScope, stats?: TCompilerStatsCompilation<T>) => IBasicModuleScope;
Expand Down
1 change: 1 addition & 0 deletions packages/rspack-test-tools/src/case/stats-output.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const creator = new BasicCaseCreator({
steps: ({ name }) => [
new StatsProcessor({
name,
writeStatsOuptut: false,
compilerType: ECompilerType.Rspack,
configFiles: ["rspack.config.js", "webpack.config.js"]
})
Expand Down
54 changes: 34 additions & 20 deletions packages/rspack-test-tools/src/processor/basic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,27 +142,41 @@ export class BasicProcessor<T extends ECompilerType> implements ITestProcessor {
const compiler = this.getCompiler(context);
const stats = compiler.getStats();
if (stats) {
fs.writeFileSync(
path.join(context.getDist(), "stats.txt"),
stats.toString({
preset: "verbose",
colors: false
}),
"utf-8"
);
const jsonStats = stats.toJson({
errorDetails: true
});
fs.writeFileSync(
path.join(context.getDist(), "stats.json"),
JSON.stringify(jsonStats, null, 2),
"utf-8"
);
if (jsonStats.errors) {
errors.push(...jsonStats.errors);
if (testConfig.writeStatsOuptut) {
fs.writeFileSync(
path.join(context.getDist(), "stats.txt"),
stats.toString({
preset: "verbose",
colors: false
}),
"utf-8"
);
}
if (jsonStats.warnings) {
warnings.push(...jsonStats.warnings);

if (testConfig.writeStatsJson) {
const jsonStats = stats.toJson({
errorDetails: true
});
fs.writeFileSync(
path.join(context.getDist(), "stats.json"),
JSON.stringify(jsonStats, null, 2),
"utf-8"
);
}

if (
fs.existsSync(context.getSource("errors.js")) ||
fs.existsSync(context.getSource("warnings.js"))
) {
const statsJson = stats.toJson({
errorDetails: true
});
if (statsJson.errors) {
errors.push(...statsJson.errors);
}
if (statsJson.warnings) {
warnings.push(...statsJson.warnings);
}
}
}
await checkArrayExpectation(
Expand Down
5 changes: 2 additions & 3 deletions packages/rspack-test-tools/src/processor/hash.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,10 @@ export class HashProcessor<
env.expect(false);
return;
}
const statsJson = stats.toJson({ assets: true });
if (REG_ERROR_CASE.test(this._options.name)) {
env.expect((statsJson.errors || []).length > 0);
env.expect(stats.hasErrors());
} else {
env.expect((statsJson.errors || []).length === 0);
env.expect(!stats.hasErrors());
}

if (typeof testConfig.validate === "function") {
Expand Down
2 changes: 1 addition & 1 deletion packages/rspack-test-tools/src/processor/hot-step.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export class HotSnapshotProcessor<

async check(env: ITestEnv, context: ITestContext) {
const compiler = this.getCompiler(context);
const stats = compiler.getStats();
const stats = compiler.getStats() as TCompilerStats<T>;
if (!stats || !stats.hash) {
env.expect(false);
return;
Expand Down
4 changes: 2 additions & 2 deletions packages/rspack-test-tools/src/processor/simple.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ export class SimpleTaskProcessor<T extends ECompilerType>

async check(env: ITestEnv, context: ITestContext) {
const compiler = this.getCompiler(context);
const stats = compiler.getStats();
const stats = compiler.getStats() as TCompilerStats<T>;
if (typeof this._options.check === "function") {
await this._options.check(context, compiler.getCompiler()!, stats!);
await this._options.check(context, compiler.getCompiler()!, stats);
}
}

Expand Down
25 changes: 21 additions & 4 deletions packages/rspack-test-tools/src/processor/snapshot.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ import type {
Compiler as WebpackCompiler
} from "webpack";

import type { ECompilerType, ITestContext, ITestEnv } from "../type";
import type {
ECompilerType,
ITestContext,
ITestEnv,
TCompilerMultiStats,
TCompilerStats
} from "../type";
import { BasicProcessor, type IBasicProcessorOptions } from "./basic";

export interface ISnapshotProcessorOptions<T extends ECompilerType>
Expand Down Expand Up @@ -33,10 +39,21 @@ export class SnapshotProcessor<
if (!stats || !c) return;

if (stats.hasErrors()) {
const errors = [];
if ((stats as TCompilerMultiStats<T>).stats) {
for (const s of (stats as TCompilerMultiStats<T>).stats) {
if (s.hasErrors()) {
errors.push(...s.compilation.errors);
}
}
} else {
const s = stats as TCompilerStats<T>;
errors.push(...s.compilation.errors);
}

throw new Error(
`Failed to compile in fixture ${this._options.name}, Errors: ${stats
.toJson({ errors: true, all: false })
.errors?.map(i => `${i.message}\n${i.stack}`)
`Failed to compile in fixture ${this._options.name}, Errors: ${errors
?.map(i => `${i.message}\n${i.stack}`)
.join("\n\n")}`
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/rspack-test-tools/src/processor/stats-api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export class StatsAPIProcessor<

async check(env: ITestEnv, context: ITestContext) {
const compiler = this.getCompiler(context);
const stats = compiler.getStats();
const stats = compiler.getStats() as TCompilerStats<T>;
env.expect(typeof stats).toBe("object");
await this._statsAPIOptions.check?.(stats!, compiler.getCompiler()!);
}
Expand Down
5 changes: 4 additions & 1 deletion packages/rspack-test-tools/src/processor/stats.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { type IMultiTaskProcessorOptions, MultiTaskProcessor } from "./multi";
export interface IStatsProcessorOptions<T extends ECompilerType>
extends Omit<IMultiTaskProcessorOptions<T>, "runable"> {
snapshotName?: string;
writeStatsOuptut?: boolean;
}

const REG_ERROR_CASE = /error$/;
Expand All @@ -28,6 +29,7 @@ export class StatsProcessor<
> extends MultiTaskProcessor<T> {
private stderr: any;
private snapshotName?: string;
private writeStatsOuptut?: boolean;
constructor(_statsOptions: IStatsProcessorOptions<T>) {
super({
defaultOptions: StatsProcessor.defaultOptions<T>,
Expand All @@ -36,6 +38,7 @@ export class StatsProcessor<
..._statsOptions
});
this.snapshotName = _statsOptions.snapshotName;
this.writeStatsOuptut = _statsOptions.writeStatsOuptut;
}

async before(context: ITestContext) {
Expand Down Expand Up @@ -114,7 +117,7 @@ export class StatsProcessor<
// errorDetails: true
})
);
} else {
} else if (this.writeStatsOuptut) {
fs.writeFileSync(
path.join(context.getDist(), "stats.txt"),
stats.toString({
Expand Down
101 changes: 70 additions & 31 deletions packages/rspack-test-tools/src/processor/watch.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import type {
ECompilerType,
ITestContext,
ITestEnv,
TCompilerOptions
TCompilerOptions,
TCompilerStatsCompilation
} from "../type";
import { type IMultiTaskProcessorOptions, MultiTaskProcessor } from "./multi";

Expand Down Expand Up @@ -102,31 +103,62 @@ export class WatchProcessor<
const checkStats = testConfig.checkStats || (() => true);

if (stats) {
fs.writeFileSync(
path.join(context.getDist(), "stats.txt"),
stats.toString({
preset: "verbose",
colors: false
}),
"utf-8"
);
const jsonStats = stats.toJson({
errorDetails: true
});
if (testConfig.writeStatsOuptut) {
fs.writeFileSync(
path.join(context.getDist(), "stats.txt"),
stats.toString({
preset: "verbose",
colors: false
}),
"utf-8"
);
}

if (!checkStats(this._watchOptions.stepName, jsonStats)) {
throw new Error("stats check failed");
const getJsonStats = (() => {
let cached: TCompilerStatsCompilation<T> | null = null;
return () => {
if (!cached) {
cached = stats.toJson({
errorDetails: true
});
}
return cached;
};
})();
if (checkStats.length > 1) {
if (!checkStats(this._watchOptions.stepName, getJsonStats())) {
throw new Error("stats check failed");
}
} else {
// @ts-expect-error only one param
if (!checkStats(this._watchOptions.stepName)) {
throw new Error("stats check failed");
}
}
fs.writeFileSync(
path.join(context.getDist(), "stats.json"),
JSON.stringify(jsonStats, null, 2),
"utf-8"
);
if (jsonStats.errors) {
errors.push(...jsonStats.errors);
if (testConfig.writeStatsJson) {
fs.writeFileSync(
path.join(context.getDist(), "stats.json"),
JSON.stringify(getJsonStats(), null, 2),
"utf-8"
);
}
if (jsonStats.warnings) {
warnings.push(...jsonStats.warnings);
if (
fs.existsSync(
context.getSource(`${this._watchOptions.stepName}/errors.js`)
) ||
fs.existsSync(
context.getSource(`${this._watchOptions.stepName}/warnings.js`)
)
) {
const statsJson = stats.toJson({
errorDetails: true
});
if (statsJson.errors) {
errors.push(...statsJson.errors);
}
if (statsJson.warnings) {
warnings.push(...statsJson.warnings);
}
}
}
await checkArrayExpectation(
Expand All @@ -151,14 +183,21 @@ export class WatchProcessor<
}

// check hash
fs.renameSync(
path.join(context.getDist(), "stats.txt"),
path.join(context.getDist(), `stats.${this._watchOptions.stepName}.txt`)
);
fs.renameSync(
path.join(context.getDist(), "stats.json"),
path.join(context.getDist(), `stats.${this._watchOptions.stepName}.json`)
);
if (testConfig.writeStatsOuptut) {
fs.renameSync(
path.join(context.getDist(), "stats.txt"),
path.join(context.getDist(), `stats.${this._watchOptions.stepName}.txt`)
);
}
if (testConfig.writeStatsJson) {
fs.renameSync(
path.join(context.getDist(), "stats.json"),
path.join(
context.getDist(),
`stats.${this._watchOptions.stepName}.json`
)
);
}
}

async config(context: ITestContext) {
Expand Down
Loading

2 comments on commit 121557a

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented on 121557a Feb 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2025-02-24 e8b77c4) Current Change
10000_big_production-mode_disable-minimize + exec 36.6 s ± 424 ms 37.3 s ± 499 ms +1.79 %
10000_development-mode + exec 1.71 s ± 27 ms 1.71 s ± 19 ms -0.13 %
10000_development-mode_hmr + exec 670 ms ± 13 ms 665 ms ± 9.3 ms -0.65 %
10000_production-mode + exec 2.18 s ± 64 ms 2.19 s ± 116 ms +0.22 %
10000_production-mode_persistent-cold + exec 2.32 s ± 60 ms 2.31 s ± 80 ms -0.85 %
10000_production-mode_persistent-hot + exec 1.63 s ± 23 ms 1.6 s ± 33 ms -1.86 %
arco-pro_development-mode + exec 1.73 s ± 129 ms 1.73 s ± 157 ms -0.01 %
arco-pro_development-mode_hmr + exec 376 ms ± 3.2 ms 376 ms ± 2.9 ms +0.05 %
arco-pro_production-mode + exec 3.58 s ± 42 ms 3.56 s ± 138 ms -0.64 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.56 s ± 197 ms 3.62 s ± 218 ms +1.43 %
arco-pro_production-mode_persistent-cold + exec 3.64 s ± 107 ms 3.64 s ± 213 ms +0.07 %
arco-pro_production-mode_persistent-hot + exec 2.23 s ± 73 ms 2.28 s ± 70 ms +2.42 %
arco-pro_production-mode_traverse-chunk-modules + exec 3.57 s ± 80 ms 3.54 s ± 56 ms -0.64 %
large-dyn-imports_development-mode + exec 1.95 s ± 40 ms 1.97 s ± 28 ms +0.83 %
large-dyn-imports_production-mode + exec 2.02 s ± 20 ms 2.05 s ± 40 ms +1.15 %
threejs_development-mode_10x + exec 1.47 s ± 65 ms 1.42 s ± 30 ms -2.98 %
threejs_development-mode_10x_hmr + exec 813 ms ± 29 ms 773 ms ± 9.4 ms -4.86 %
threejs_production-mode_10x + exec 5.05 s ± 119 ms 5 s ± 51 ms -1.09 %
threejs_production-mode_10x_persistent-cold + exec 5.16 s ± 452 ms 5.11 s ± 168 ms -0.86 %
threejs_production-mode_10x_persistent-hot + exec 4.52 s ± 334 ms 4.44 s ± 313 ms -1.76 %
10000_big_production-mode_disable-minimize + rss memory 8678 MiB ± 53.9 MiB 8788 MiB ± 286 MiB +1.26 %
10000_development-mode + rss memory 648 MiB ± 37 MiB 637 MiB ± 22.6 MiB -1.70 %
10000_development-mode_hmr + rss memory 1268 MiB ± 261 MiB 1303 MiB ± 143 MiB +2.79 %
10000_production-mode + rss memory 618 MiB ± 6.5 MiB 623 MiB ± 25.6 MiB +0.88 %
10000_production-mode_persistent-cold + rss memory 727 MiB ± 11.4 MiB 729 MiB ± 16.3 MiB +0.25 %
10000_production-mode_persistent-hot + rss memory 700 MiB ± 10.1 MiB 701 MiB ± 12.8 MiB +0.05 %
arco-pro_development-mode + rss memory 567 MiB ± 40.7 MiB 566 MiB ± 34.7 MiB -0.24 %
arco-pro_development-mode_hmr + rss memory 652 MiB ± 75.8 MiB 639 MiB ± 71.6 MiB -1.97 %
arco-pro_production-mode + rss memory 714 MiB ± 18.6 MiB 692 MiB ± 22.9 MiB -3.08 %
arco-pro_production-mode_generate-package-json-webpack-plugin + rss memory 728 MiB ± 30.3 MiB 708 MiB ± 11.8 MiB -2.67 %
arco-pro_production-mode_persistent-cold + rss memory 785 MiB ± 29.3 MiB 771 MiB ± 38.1 MiB -1.85 %
arco-pro_production-mode_persistent-hot + rss memory 640 MiB ± 28.5 MiB 644 MiB ± 25.7 MiB +0.70 %
arco-pro_production-mode_traverse-chunk-modules + rss memory 717 MiB ± 23.4 MiB 719 MiB ± 27.6 MiB +0.16 %
large-dyn-imports_development-mode + rss memory 643 MiB ± 4.44 MiB 642 MiB ± 4.92 MiB -0.14 %
large-dyn-imports_production-mode + rss memory 523 MiB ± 3.78 MiB 521 MiB ± 5.85 MiB -0.33 %
threejs_development-mode_10x + rss memory 569 MiB ± 19.6 MiB 555 MiB ± 26.3 MiB -2.38 %
threejs_development-mode_10x_hmr + rss memory 1207 MiB ± 75.4 MiB 1123 MiB ± 214 MiB -6.99 %
threejs_production-mode_10x + rss memory 847 MiB ± 44.7 MiB 848 MiB ± 33 MiB +0.07 %
threejs_production-mode_10x_persistent-cold + rss memory 969 MiB ± 48 MiB 957 MiB ± 63.7 MiB -1.24 %
threejs_production-mode_10x_persistent-hot + rss memory 802 MiB ± 39.1 MiB 787 MiB ± 37.4 MiB -1.89 %

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented on 121557a Feb 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

📝 Ecosystem CI detail: Open

suite result
modernjs ❌ failure
rspress ✅ success
rslib ✅ success
rsbuild ❌ failure
rsdoctor ❌ failure
examples ✅ success
devserver ✅ success
nuxt ✅ success

Please sign in to comment.