Skip to content

Commit

Permalink
Make the test timeout always command-line configurable for YAML tests. (
Browse files Browse the repository at this point in the history
#15973)

A test's timeout was only configurable if "timeout" was set in the
YAML.  Make sure the codegen acts as if that is always set.

Also modifies zap_regen_all.py to clearly list all the things it's
planning to run to generate things up front, and adds chip-tool-darwin
to the set of things it generates.

Also adds a --dry-run option to zap_regen_all.py to just list the
targets and exit.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed May 27, 2022
1 parent 34258d2 commit 1075586
Show file tree
Hide file tree
Showing 12 changed files with 3,007 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,7 @@ class {{filename}}: public TestCommandBridge
}
}

{{#if timeout}}
chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(mTimeout.HasValue() ? mTimeout.Value() : {{timeout}}); }
{{/if}}
chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(mTimeout.ValueOr({{chip_tests_config_get_default_value "timeout"}})); }

private:
std::atomic_uint16_t mTestIndex;
Expand Down
5 changes: 0 additions & 5 deletions examples/chip-tool/commands/tests/TestCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,12 @@ class TestCommand : public CHIPCommand,
{
AddArgument("delayInMs", 0, UINT64_MAX, &mDelayInMs);
AddArgument("PICS", &mPICSFilePath);
AddArgument("testTimeoutInS", 0, UINT16_MAX, &mTimeout);
}

~TestCommand(){};

/////////// CHIPCommand Interface /////////
CHIP_ERROR RunCommand() override;
chip::System::Clock::Timeout GetWaitDuration() const override
{
return chip::System::Clock::Seconds16(mTimeout.ValueOr(kTimeoutInSeconds));
}
virtual void NextTest() = 0;

protected:
Expand Down
4 changes: 2 additions & 2 deletions examples/chip-tool/templates/tests/commands.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ public:
}
};

{{>test_cluster tests=(getTests) credsIssuerConfigArg=true}}
{{>test_cluster tests=(getManualTests) credsIssuerConfigArg=true}}
{{>test_cluster tests=(getTests) credsIssuerConfigArg=true needsWaitDuration=true}}
{{>test_cluster tests=(getManualTests) credsIssuerConfigArg=true needsWaitDuration=true}}

void registerCommandsTests(Commands & commands, CredentialIssuerCommands * credsIssuerConfig)
{
Expand Down
4 changes: 2 additions & 2 deletions examples/chip-tool/templates/tests/partials/test_cluster.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ class {{filename}}Suite: public TestCommand
}
}

{{#if timeout}}
chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(mTimeout.HasValue() ? mTimeout.Value() : {{timeout}}); }
{{#if ../needsWaitDuration}}
chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(mTimeout.ValueOr({{chip_tests_config_get_default_value "timeout"}})); }
{{/if}}

private:
Expand Down
2 changes: 1 addition & 1 deletion examples/placeholder/templates/tests-commands.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "TestCommand.h"

{{#if (getTests)}}
{{>test_cluster tests=(getTests) credsIssuerConfigArg=false}}
{{>test_cluster tests=(getTests) credsIssuerConfigArg=false needsWaitDuration=false}}
{{/if}}

std::unique_ptr<TestCommand>GetTestCommand(std::string testName)
Expand Down
31 changes: 26 additions & 5 deletions scripts/tools/zap_regen_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,13 @@ def __init__(self, zap_config, template=None, output_dir=None):
else:
self.output_dir = None

def generate(self):
"""Runs a ZAP generate command on the configured zap/template/outputs.
def log_command(self):
"""Log the command that will get run for this target
"""
logging.info(" %s" % " ".join(self.build_cmd()))

def build_cmd(self):
"""Builds the command line we would run to generate this target.
"""
cmd = [self.script, self.zap_config]

Expand All @@ -53,6 +58,12 @@ def generate(self):
cmd.append('-o')
cmd.append(self.output_dir)

return cmd

def generate(self):
"""Runs a ZAP generate command on the configured zap/template/outputs.
"""
cmd = self.build_cmd()
logging.info("Generating target: %s" % " ".join(cmd))
subprocess.check_call(cmd)

Expand All @@ -71,6 +82,8 @@ def setupArgumentsParser():
help='Choose which content type to generate (default: all)')
parser.add_argument('--tests', default='all', choices=['all', 'chip-tool', 'darwin', 'app1', 'app2'],
help='When generating tests only target, Choose which tests to generate (default: all)')
parser.add_argument('--dry-run', default=False, action='store_true',
help="Don't do any generationl just log what targets would be generated (default: False)")
return parser.parse_args()


Expand Down Expand Up @@ -112,7 +125,7 @@ def getGlobalTemplatesTargets():
targets.append(ZAPGenerateTarget(filepath, output_dir=output_dir))

targets.append(ZAPGenerateTarget(
'./src/controller/data_model/controller-clusters.zap',
'src/controller/data_model/controller-clusters.zap',
output_dir=os.path.join('zzz_generated/controller-clusters/zap-generated')))

return targets
Expand Down Expand Up @@ -163,6 +176,7 @@ def getSpecificTemplatesTargets():
templates = {
'src/app/common/templates/templates.json': 'zzz_generated/app-common/app-common/zap-generated',
'examples/chip-tool/templates/templates.json': 'zzz_generated/chip-tool/zap-generated',
'examples/chip-tool-darwin/templates/templates.json': 'zzz_generated/chip-tool-darwin/zap-generated',
'src/controller/python/templates/templates.json': None,
'src/darwin/Framework/CHIP/templates/templates.json': None,
'src/controller/java/templates/templates.json': None,
Expand All @@ -171,6 +185,7 @@ def getSpecificTemplatesTargets():

targets = []
for template, output_dir in templates.items():
logging.info("Found specific template %s" % template)
targets.append(ZAPGenerateTarget(
zap_filepath, template=template, output_dir=output_dir))

Expand All @@ -187,6 +202,10 @@ def getTargets(type, test_target):
elif type == 'tests':
targets.extend(getTestsTemplatesTargets(test_target))

logging.info("Targets to be generated:")
for target in targets:
target.log_command()

return targets


Expand All @@ -200,8 +219,10 @@ def main():
args = setupArgumentsParser()

targets = getTargets(args.type, args.tests)
for target in targets:
target.generate()

if (not args.dry_run):
for target in targets:
target.generate()


if __name__ == '__main__':
Expand Down
1 change: 0 additions & 1 deletion src/app/zap-templates/common/ClusterTestGeneration.js
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,6 @@ function parse(filename)
});

yaml.filename = filename;
yaml.timeout = yaml.config.timeout;
yaml.totalTests = yaml.tests.length;

return yaml;
Expand Down
10 changes: 9 additions & 1 deletion src/app/zap-templates/common/variables/Variables.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const knownVariables = {
'nodeId' : { type : 'NODE_ID', defaultValue : 0x12345 },
'endpoint' : { type : 'ENDPOINT_NO', defaultValue : '' },
'cluster' : { type : 'CHAR_STRING', defaultValue : '' },
'timeout' : { type : 'INT16U', defaultValue : 30 },
'timeout' : { type : 'INT16U', defaultValue : "kTimeoutInSeconds" },
};

function throwError(test, errorStr)
Expand Down Expand Up @@ -70,6 +70,14 @@ async function extractVariablesFromConfig(context, suite)
{
let variables = [];

// Ensure that timeout is always set in the config, to enable command-line
// control over it.
if (!("timeout" in suite.config)) {
// Set to the defaultValue, because below for the isKnownVariable case we will use
// the actual value as the default value...
suite.config.timeout = knownVariables.timeout.defaultValue;
}

for (const key in suite.config) {
let value = {};

Expand Down
Loading

0 comments on commit 1075586

Please sign in to comment.