Skip to content

Re-add support for suite params#379

Merged
andrewvc merged 5 commits intoelastic:masterfrom
andrewvc:bring-back-suite-params
Sep 16, 2021
Merged

Re-add support for suite params#379
andrewvc merged 5 commits intoelastic:masterfrom
andrewvc:bring-back-suite-params

Conversation

@andrewvc
Copy link
Copy Markdown
Contributor

Heartbeat 7.15/7.x did not get elastic/beats#26674 backported to them. Since they will be released soon we need to work on re-supporting the --suite-params flag and do a new release once this is merged.

@andrewvc andrewvc added the bug Something isn't working label Sep 16, 2021
@andrewvc andrewvc self-assigned this Sep 16, 2021
@ghost
Copy link
Copy Markdown

ghost commented Sep 16, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-09-16T22:30:27.876+0000

  • Duration: 15 min 32 sec

  • Commit: 6929fed

Test stats 🧪

Test Results
Failed 0
Passed 132
Skipped 0
Total 132

Trends 🧪

Image of Build Times

Image of Tests

Copy link
Copy Markdown
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

code LGTM.

)
.option(
'-p, --params <jsonstring>',
'-p, --params <jsonstring>, --suite-params <jsonstring>',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am confused with this notation, What does this do?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops, let me remove it. It does nothing. You can't add a third form of the command, I needed to add a new option below.

Copy link
Copy Markdown
Member

@vigneshshanmugam vigneshshanmugam left a comment

Choose a reason for hiding this comment

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

Overall I feel like we can take the changes from the previous PR https://github.com/elastic/synthetics/pull/331/files and add the test we have here.

Copy link
Copy Markdown
Member

@vigneshshanmugam vigneshshanmugam left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewvc andrewvc merged commit e7ccc5c into elastic:master Sep 16, 2021
@andrewvc andrewvc deleted the bring-back-suite-params branch September 16, 2021 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants