Skip to content

Run rustdoc-gui tests in parallel #86692

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

Merged
merged 8 commits into from
Aug 15, 2021
Merged
Changes from 6 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
113 changes: 100 additions & 13 deletions src/tools/rustdoc-gui/tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,45 @@ function parseOptions(args) {
return null;
}

/// Print single char status information without \n
function char_printer(n_tests) {
const max_per_line = 10;
let current = 0;
return {
successful: function() {
current += 1;
if (current % max_per_line === 0) {
process.stdout.write(`. (${current}/${n_tests})\n`);
} else {
process.stdout.write(".");
}
},
erroneous: function() {
current += 1;
if (current % max_per_line === 0) {
process.stderr.write(`F (${current}/${n_tests})\n`);
} else {
process.stderr.write("F");
}
},
};
}

/// Sort array by .file_name property
function by_filename(a, b) {
return a.file_name - b.file_name;
}

async function main(argv) {
let opts = parseOptions(argv.slice(2));
if (opts === null) {
process.exit(1);
}

// Print successful tests too
let debug = false;
// Run tests in sequentially
let no_headless = false;
const options = new Options();
try {
// This is more convenient that setting fields one by one.
Expand All @@ -84,13 +117,15 @@ async function main(argv) {
"--variable", "DOC_PATH", opts["doc_folder"],
];
if (opts["debug"]) {
debug = true;
args.push("--debug");
}
if (opts["show_text"]) {
args.push("--show-text");
}
if (opts["no_headless"]) {
args.push("--no-headless");
no_headless = true;
}
options.parseArguments(args);
} catch (error) {
Expand All @@ -101,25 +136,77 @@ async function main(argv) {
let failed = false;
let files;
if (opts["files"].length === 0) {
files = fs.readdirSync(opts["tests_folder"]).filter(file => path.extname(file) == ".goml");
files = fs.readdirSync(opts["tests_folder"]);
} else {
files = opts["files"].filter(file => path.extname(file) == ".goml");
files = opts["files"];
}
files = files.filter(file => path.extname(file) == ".goml");
if (files.length === 0) {
console.error("rustdoc-gui: No test selected");
process.exit(2);
}

files.sort();
for (var i = 0; i < files.length; ++i) {
const testPath = path.join(opts["tests_folder"], files[i]);
await runTest(testPath, options).then(out => {
const [output, nb_failures] = out;
console.log(output);
if (nb_failures > 0) {

console.log(`Running ${files.length} rustdoc-gui tests...`);
process.setMaxListeners(files.length + 1);
let tests = [];
let results = {
successful: [],
failed: [],
errored: [],
};
const status_bar = char_printer(files.length);
for (let i = 0; i < files.length; ++i) {
const file_name = files[i];
const testPath = path.join(opts["tests_folder"], file_name);
tests.push(
runTest(testPath, options)
.then(out => {
const [output, nb_failures] = out;
results[nb_failures === 0 ? "successful" : "failed"].push({
file_name: file_name,
output: output,
});
if (nb_failures > 0) {
status_bar.erroneous()
failed = true;
} else {
status_bar.successful()
}
})
.catch(err => {
results.errored.push({
file_name: file_name,
output: err,
});
status_bar.erroneous();
failed = true;
}
}).catch(err => {
console.error(err);
failed = true;
})
);
if (no_headless) {
await tests[i];
}
}
await Promise.all(tests);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be:

Suggested change
await Promise.all(tests);
if (!no_headless) {
await Promise.all(tests);
}

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Logic wise kind of yes. But you can await a future multiple times, the work will get done only once

// final \n after the tests
console.log("\n");
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be:

Suggested change
console.log("\n");
console.log("");

? Or do you specifically want an empty line?

Copy link
Member

Choose a reason for hiding this comment

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

If so, maybe better to use two console.log("") instead I think. No strong opinion though.

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 wanted a bit more separation between the sections for readability so the progress bar is visually spaced from the outputs.
The output should be the same on all platforms because we don't write anything on the empty line but fully correct it should be two console.log(""), will change that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced it with a better finish method that prints the final stats


if (debug === false) {
results.successful.sort(by_filename);
results.successful.forEach(r => {
console.log(r.output);
});
}
results.failed.sort(by_filename);
results.failed.forEach(r => {
console.log(r.output);
});
// print run errors on the bottom so developers see them better
results.errored.sort(by_filename);
results.errored.forEach(r => {
console.error(r.output);
});

if (failed) {
process.exit(1);
}
Expand Down