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

Add argument & command validation in scope of repo #234

Merged
merged 4 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
79 changes: 53 additions & 26 deletions src/bot/parse/parsePullRequestBotCommandLine.namedArgs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type DataProvider = {
suitName: string;
commandLine: string;
expectedResponse: SkipEvent | ParsedCommand | Error;
repo?: string;
};

const dataProvider: DataProvider[] = [
Expand All @@ -24,7 +25,7 @@ const dataProvider: DataProvider[] = [
"bench",
{
commandStart: ['"$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh"'],
gitlab: { job: { tags: ["bench-bot"], variables: {} } },
gitlab: { job: { tags: ["weights-vm"], variables: {} } },
},
{},
'"$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=runtime --runtime=polkadot --target_dir=polkadot --pallet=pallet_referenda',
Expand All @@ -38,20 +39,21 @@ const dataProvider: DataProvider[] = [
"bench",
{
commandStart: ['"$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh"'],
gitlab: { job: { tags: ["bench-bot"], variables: {} } },
gitlab: { job: { tags: ["weights-vm"], variables: {} } },
},
{ PIPELINE_SCRIPTS_REF: "branch" },
'"$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=bridge-hub-kusama --runtime_dir=bridge-hubs --target_dir=cumulus --pallet=pallet_name',
),
repo: "cumulus",
},
{
suitName: "unrelated to bot comment returns nothing (ignores)",
commandLine: "something from comments",
expectedResponse: new SkipEvent("Not a command"),
},
{
suitName: "try-runtime-bot with default preset mentioned explicitly",
commandLine: "bot try-runtime -v RUST_LOG=remote-ext=debug,runtime=trace -v SECOND=val default --chain=kusama",
suitName: "try-runtime-bot testing default without mentioning preset name",
commandLine: "bot try-runtime -v RUST_LOG=remote-ext=debug,runtime=trace -v SECOND=val --chain=kusama",
expectedResponse: new GenericCommand(
"try-runtime",
{
Expand All @@ -63,30 +65,54 @@ const dataProvider: DataProvider[] = [
),
},
{
suitName: "try-runtime-bot testing default without mentioning preset name",
commandLine: "bot try-runtime -v RUST_LOG=remote-ext=debug,runtime=trace -v SECOND=val --chain=kusama",
suitName: "try-runtime-bot testing default without any args",
commandLine: "bot try-runtime",
expectedResponse: new GenericCommand(
"try-runtime",
{
commandStart: ['"$PIPELINE_SCRIPTS_DIR/commands/try-runtime/try-runtime.sh"'],
gitlab: { job: { tags: ["linux-docker-vm-c2"], variables: {} } },
},
{ RUST_LOG: "remote-ext=debug,runtime=trace", SECOND: "val" },
'"$PIPELINE_SCRIPTS_DIR/commands/try-runtime/try-runtime.sh" --chain=kusama --target_path=. --chain_node=polkadot',
{},
'"$PIPELINE_SCRIPTS_DIR/commands/try-runtime/try-runtime.sh" --chain=polkadot --target_path=. --chain_node=polkadot',
),
},
{
suitName: "try-runtime-bot testing default without any args",
commandLine: "bot try-runtime",
suitName: "try-runtime-bot testing wrong presets",
commandLine: "bot try-runtime unbelievable",
expectedResponse: new Error(
`Unknown subcommand of "try-runtime". Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).`,
),
},
{
suitName: "try-runtime-bot testing trappist",
commandLine: "bot try-runtime trappist",
expectedResponse: new GenericCommand(
"try-runtime",
{
commandStart: ['"$PIPELINE_SCRIPTS_DIR/commands/try-runtime/try-runtime.sh"'],
gitlab: { job: { tags: ["linux-docker-vm-c2"], variables: {} } },
},
{},
'"$PIPELINE_SCRIPTS_DIR/commands/try-runtime/try-runtime.sh" --chain=polkadot --target_path=. --chain_node=polkadot',
'"$PIPELINE_SCRIPTS_DIR/commands/try-runtime/try-runtime.sh" --chain=trappist --chain_node=trappist-node --target_path=. --live_uri=rococo-trappist',
),
repo: "trappist",
},
{
suitName: "try-runtime-bot testing trappist with existing but unsupported preset [default]",
commandLine: "bot try-runtime",
expectedResponse: new Error(
`Missing arguments for command "try-runtime". Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).`,
),
repo: "trappist",
},
{
suitName: "try-runtime-bot testing trappist with existing but unsupported preset [polkadot]",
commandLine: "bot try-runtime polkadot",
expectedResponse: new Error(
`Unknown subcommand of "try-runtime". Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).`,
),
repo: "trappist",
},
{
suitName: "fmt, no args should be allowed and return config",
Expand Down Expand Up @@ -142,7 +168,7 @@ const dataProvider: DataProvider[] = [
},
{
suitName: "command, with 'default' preset should add properly",
commandLine: "bot sample default --input=bla",
commandLine: "bot sample --input=bla",
expectedResponse: new GenericCommand(
"sample",
{
Expand All @@ -168,8 +194,8 @@ const dataProvider: DataProvider[] = [
},

/*
Help cases
*/
Help cases
*/
{
suitName: "help",
commandLine: "bot help",
Expand All @@ -184,14 +210,14 @@ const dataProvider: DataProvider[] = [
{ suitName: "clear", commandLine: "bot clear", expectedResponse: new CleanCommand() },

/*
Cancel cases
*/
Cancel cases
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Automatic formatting with IDEA? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

most probably :)

{ suitName: "cancel no-taskId", commandLine: "bot cancel", expectedResponse: new CancelCommand("") },
{ suitName: "cancel with taskId", commandLine: "bot cancel 123123", expectedResponse: new CancelCommand("123123") },

/*
Ignore cases
*/
Ignore cases
*/
{
suitName: "empty command line returns nothing (ignores)",
commandLine: "",
Expand Down Expand Up @@ -220,8 +246,8 @@ const dataProvider: DataProvider[] = [
},

/*
Expected Error cases
*/
Expected Error cases
*/
{
suitName: "bench-bot --pallet should validate the matching rule",
commandLine: "bot bench polkadot-pallet --pallet=00034",
Expand All @@ -240,6 +266,7 @@ const dataProvider: DataProvider[] = [
suitName: "bench-bot, no required args, should return error",
commandLine: "bot bench cumulus-bridge-hubs",
expectedResponse: new Error(`required option '--pallet <value>' not specified`),
repo: "cumulus",
},
{
suitName: "sample without required arg should return error",
Expand All @@ -257,21 +284,21 @@ const dataProvider: DataProvider[] = [
suitName: "nonexistent command, should return proper error",
commandLine: "bot nope 123123",
expectedResponse: new Error(
'Unknown command "nope"; Available ones are bench-all, bench-overhead, bench-vm, bench, fmt, merge, rebase, sample, try-runtime, update-ui. Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
'Unknown command "nope". Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
),
},
{
suitName: "not provided command, returns proper error",
commandLine: "bot $",
expectedResponse: new Error(
'Unknown command "$"; Available ones are bench-all, bench-overhead, bench-vm, bench, fmt, merge, rebase, sample, try-runtime, update-ui. Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
'Unknown command "$". Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
),
},
{
suitName: "non existed config must return error with explanation",
commandLine: "bot xz",
expectedResponse: new Error(
`Unknown command "xz"; Available ones are bench-all, bench-overhead, bench-vm, bench, fmt, merge, rebase, sample, try-runtime, update-ui. Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).`,
`Unknown command "xz". Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).`,
),
},
{
Expand All @@ -291,9 +318,9 @@ const dataProvider: DataProvider[] = [
];

describe("parsePullRequestBotCommandLine", () => {
for (const { suitName, commandLine, expectedResponse } of dataProvider) {
test(`test commandLine: ${commandLine} [${suitName}]`, async () => {
const res = await parsePullRequestBotCommandLine(commandLine, { logger }, "polkadot");
for (const { suitName, commandLine, expectedResponse, repo } of dataProvider) {
test(`test commandLine: "${commandLine}" [${suitName}] ${repo ? "repo: [" + repo + "]" : ""}`, async () => {
const res = await parsePullRequestBotCommandLine(commandLine, { logger }, repo || "polkadot");
expect(res).toEqual(expectedResponse);
});
}
Expand Down
8 changes: 4 additions & 4 deletions src/bot/parse/parsePullRequestBotCommandLine.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const dataProvider: DataProvider[] = [
"bench",
{
commandStart: ['"$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh"'],
gitlab: { job: { tags: ["bench-bot"], variables: {} } },
gitlab: { job: { tags: ["weights-vm"], variables: {} } },
},
{ PIPELINE_SCRIPTS_REF: "hello-is-this-even-used" },
'"$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime kusama-dev pallet_referenda',
Expand Down Expand Up @@ -130,21 +130,21 @@ const dataProvider: DataProvider[] = [
suitName: "nonexistent command, should return proper error",
commandLine: "bot nope 123123",
expectedResponse: new Error(
'Unknown command "nope"; Available ones are bench-all, bench-overhead, bench-vm, bench, fmt, merge, rebase, sample, try-runtime, update-ui. Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
'Unknown command "nope". Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
),
},
{
suitName: "not provided command, returns proper error",
commandLine: "bot $",
expectedResponse: new Error(
'Unknown command "$"; Available ones are bench-all, bench-overhead, bench-vm, bench, fmt, merge, rebase, sample, try-runtime, update-ui. Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
'Unknown command "$". Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
),
},
{
suitName: "non existed config must return error with explanation",
commandLine: "bot xz",
expectedResponse: new Error(
`Unknown command "xz"; Available ones are bench-all, bench-overhead, bench-vm, bench, fmt, merge, rebase, sample, try-runtime, update-ui. Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).`,
`Unknown command "xz". Refer to [help docs](http://cmd-bot.docs.com/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).`,
),
},
];
Expand Down
35 changes: 21 additions & 14 deletions src/command-configs/__mocks__/fetchCommandsConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,30 @@ export const cmd: CommandConfigs = {
trappist: {
description: "Pallet Benchmark for Trappist",
repos: ["trappist"],
args: { runtime: { label: "Runtime", type_one_of: ["trappist", "stout"] } },
args: {
runtime: { label: "Runtime", type_one_of: ["trappist", "stout"] },
target_dir: { label: "Target Directory", type_string: "trappist" },
},
},
},
},
},
"bench-bm": {
$schema: "../../node_modules/command-bot/src/schema/schema.cmd.json",
command: {
description: "This is a testing for `bench` command running on legacy BM machines",
configuration: {
gitlab: { job: { tags: ["weights-vm"], variables: {} } },
commandStart: ['"$PIPELINE_SCRIPTS_DIR/commands/bench-bm/bench-bm.sh"'],
},
},
},
"bench-overhead": {
$schema: "../../node_modules/command-bot/src/schema/schema.cmd.json",
command: {
description: "Run benchmarks overhead and commit back results to PR",
configuration: {
gitlab: { job: { tags: ["bench-bot"], variables: {} } },
gitlab: { job: { tags: ["weights-vm"], variables: {} } },
commandStart: ['"$PIPELINE_SCRIPTS_DIR/commands/bench-overhead/bench-overhead.sh"'],
},
presets: {
Expand Down Expand Up @@ -76,22 +89,12 @@ export const cmd: CommandConfigs = {
},
},
},
"bench-vm": {
$schema: "../../node_modules/command-bot/src/schema/schema.cmd.json",
command: {
description: "This is a testing for `bench` command running on VM machine",
configuration: {
gitlab: { job: { tags: ["weights-vm"], variables: {} } },
commandStart: ['"$PIPELINE_SCRIPTS_DIR/commands/bench-vm/bench-vm.sh"'],
},
},
},
bench: {
$schema: "../../node_modules/command-bot/src/schema/schema.cmd.json",
command: {
description: "Runs `benchmark pallet` or `benchmark overhead` against your PR and commits back updated weights",
configuration: {
gitlab: { job: { tags: ["bench-bot"], variables: {} } },
gitlab: { job: { tags: ["weights-vm"], variables: {} } },
commandStart: ['"$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh"'],
},
presets: {
Expand Down Expand Up @@ -202,6 +205,7 @@ export const cmd: CommandConfigs = {
subcommand: { label: "Subcommand", type_one_of: ["runtime", "xcm"] },
runtime: { label: "Runtime", type_one_of: ["trappist", "stout"] },
pallet: { label: "Pallet", type_rule: "^([a-z_]+)([:]{2}[a-z_]+)?$", example: "pallet_name" },
target_dir: { label: "Target Directory", type_string: "trappist" },
},
},
},
Expand All @@ -226,6 +230,7 @@ export const cmd: CommandConfigs = {
gitlab: { job: { tags: [""] } },
commandStart: ['"$PIPELINE_SCRIPTS_DIR/commands/merge/merge.sh"'],
},
presets: { default: { description: "merge PR", repos: ["substrate", "polkadot", "cumulus"] } },
},
},
rebase: {
Expand All @@ -234,9 +239,10 @@ export const cmd: CommandConfigs = {
description:
"create a merge commit from the target branch into the PR. Read more: https://github.com/paritytech/parity-processbot/",
configuration: {
gitlab: { job: { tags: [""] } },
gitlab: { job: { tags: [""], variables: {} } },
commandStart: ['"$PIPELINE_SCRIPTS_DIR/commands/rebase/rebase.sh"'],
},
presets: { default: { description: "pull latest from the base", repos: ["substrate", "polkadot", "cumulus"] } },
},
},
sample: {
Expand Down Expand Up @@ -291,6 +297,7 @@ export const cmd: CommandConfigs = {
chain: { label: "Chain", type_one_of: ["trappist"] },
chain_node: { label: "Chain Node", type_string: "trappist-node" },
target_path: { label: "Target Path", type_string: "." },
live_uri: { label: "Live Node URI ID", type_string: "rococo-trappist" },
},
},
},
Expand Down
12 changes: 2 additions & 10 deletions src/command-configs/renderHelpPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import path from "path";
import * as pug from "pug";

import { CommandConfigs } from "src/command-configs/types";
import { getSupportedRepoNames } from "src/command-configs/utils";
import { Config } from "src/config";
import { CmdJson } from "src/schema/schema.cmd";

Expand All @@ -19,16 +20,7 @@ export function renderHelpPage(params: {

const preparedConfigs = prepareConfigs(commandConfigs);

// getting list of possible repos, to be able to filter out relevant commands
const reposSet = new Set<string>();
for (const cmdConfig of Object.values(preparedConfigs)) {
for (const preset of Object.values(cmdConfig.command.presets ?? {})) {
for (const repo of preset.repos ?? []) {
reposSet.add(repo);
}
}
}
const repos = [...reposSet];
const repos = getSupportedRepoNames(preparedConfigs).repos;

// TODO: depends on headBranch, if overridden: add `-v PIPELINE_SCRIPTS_REF=branch` to all command examples same for PATCH_repo=xxx
// TODO: Simplify the PIPELINE_SCRIPTS_REF to something more rememberable */
Expand Down
Loading
Loading