Skip to content

Commit

Permalink
Add argument & command validation in scope of repo (#234)
Browse files Browse the repository at this point in the history
* Add argument & command validation in scope of repo

* Update src/command-configs/utils.ts

Co-authored-by: Yuri Volkov <0@mcornholio.ru>
Signed-off-by: Maksym H <1177472+mordamax@users.noreply.github.com>

* Update commander.ts

---------

Signed-off-by: Maksym H <1177472+mordamax@users.noreply.github.com>
Co-authored-by: Yuri Volkov <0@mcornholio.ru>
  • Loading branch information
mordamax and mutantcornholio committed Aug 23, 2023
1 parent e33d13d commit a50ccde
Show file tree
Hide file tree
Showing 9 changed files with 259 additions and 137 deletions.
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
*/
{ 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

0 comments on commit a50ccde

Please sign in to comment.