Skip to content

Conversation

@jackkav
Copy link
Contributor

@jackkav jackkav commented Jul 11, 2024

  • works inso run collection
  • logging
  • request list by workspaceId should run all requests not just top level requests
  • fixtures should point at local server

future work

  • example fixtures of bugs and things to build
  • request group selection
  • scripting

thoughts

  • it would be nice to add a folder selection step but asks a lot of traversal problems so will scope it out

@jackkav jackkav marked this pull request as draft July 11, 2024 05:22
@jackkav jackkav changed the title collection-runner poc feat: inso collection runner Jul 11, 2024

let requests = db.Request.filter(req => req.parentId === workspaceId);
if (options.requestNamePattern) {
requests = requests.filter(req => req.name.match(new RegExp(options.requestNamePattern)));

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.lang.security.audit.detect-non-literal-regexp.detect-non-literal-regexp

RegExp() called with a `cmd` function argument, this might allow an attacker to cause a Regular Expression Denial-of-Service (ReDoS) within your application as RegExP blocks the main thread. For this reason, it is recommended to use hardcoded regexes instead. If your regex is run on user-controlled input, consider performing input validation or use a regex checking/sanitization library such as https://www.npmjs.com/package/recheck to verify that the regex does not appear vulnerable to ReDoS.
@jackkav jackkav marked this pull request as ready for review July 16, 2024 12:57
@jackkav jackkav force-pushed the feature/ins-4143-basic-collection-runner branch from 6083765 to db642a8 Compare July 16, 2024 12:58
@jackkav jackkav requested a review from a team July 17, 2024 08:29
Copy link
Contributor

@ihexxa ihexxa left a comment

Choose a reason for hiding this comment

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

Just added some minor comments.

const options = {
reporter: defaultReporter,
...__configFile?.options || {},
...commandOptions,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: just double check that command options will be overridden by the config file? Original I thought command options might have highest priority.

return process.exit(1);
}

const db = await loadDb({
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably we could be a bit defensive here and check loadDb failure.

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 agree the db load logic is over complex I'm thinking of a better way to abstract it. Will follow up with a pr soon

logger.log(`Running request: ${req.name} ${req._id}`);
const res = await sendRequest(req._id);
logger.trace(res);
if (res.status !== 200) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Just a todo item that we might check test results to decide if it is failed in the future.

@jackkav jackkav merged commit b0b6c20 into Kong:develop Jul 17, 2024
@jackkav jackkav deleted the feature/ins-4143-basic-collection-runner branch July 17, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants