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 new "st2 action-alias execute-and-format" command for easier ChatOps alias development and testing #5143

Merged
merged 13 commits into from
Mar 27, 2022

Conversation

Kami
Copy link
Member

@Kami Kami commented Feb 8, 2021

This pull request adds new st2 action-alias execute-and-format (open to a better name) command which should make testing ChatOps aliases easier.

It allows user to simulate ChatOps command using the CLI by performing the match and rendering / formatting + printing the result.
For example:

# 1. No matching aliases found
$ st2 action-alias execute-and-render "server stats foo" 
Triggering execution via action alias

ERROR: 400 Client Error: Bad Request
MESSAGE: Command 'server stats foo' matched no patterns for url: http://st2api:9101/aliasexecution/match_and_execute


# 2. Matching alias found
$ st2 action-alias execute-and-render "server stats" 
Triggering execution via action alias

Execution (6021aa3c0a115899922742a3) has been started, waiting for it to finish...

.

Execution (6021aa3c0a115899922742a3) has finished, rendering result...

Execution (6021aa3e0a115899922742a6) has been started, waiting for it to finish...

.

Formatted ChatOps result message

================================================================================
Uptime: 2 days :chart_with_upwards_trend: 
Load Average (1 min, 15 min): 272721.806, 0.282
Memory Usage: (free / total): 1.32 GB / 62.54 GB 

CPUs: 

Core 0: 38 C :large_green_circle: 
Core 1: 39 C :large_green_circle: 
Core 2: 39 C :large_green_circle: 
Core 3: 38 C :large_green_circle: 

Disks (SSDs):

Disk 0: 28 C :large_green_circle: 
Disk 1: 27 C :large_green_circle: 
Disk 2: 27 C :large_green_circle: 
Disk 3: 26 C :large_green_circle: 
================================================================================

Background, Context

Currently StackStorm offers match and execute commands. The latter performs matching and if a matching alias is found, parameters are parsed and new action execution is created.

That's all good since matching a command is usually first part of developing and troubleshooting an alias. But second part of developing an alias is verifying returned data is formatted correctly.

Right now there is no easy way to test that, but to do it directly in the chat which can be slow and cumbersome.

This proposed changes improves on that story that by adding a new command which allows user to simulate ChatOps actions via StackStorm CLI.

TODO

to execute an action alias and render the result and display it.

This will display the same message which would otherwise have been
displayed in chat.

This should make testing various aliases easier and faster.
@pull-request-size pull-request-size bot added the size/M PR that changes 30-99 lines. Good size to review. label Feb 8, 2021
@arm4b arm4b added this to the 3.5.0 milestone Feb 8, 2021
@Kami
Copy link
Member Author

Kami commented Feb 8, 2021

@StackStorm/maintainers any suggestions for a better command line option name? Maybe execute-and-print or just run?

@arm4b
Copy link
Member

arm4b commented Feb 9, 2021

I assume it's useful for testing purposes.

Maybe just st2 action-alias test? I liked st2 action-alias run too, but both might be too broad?
st2 action-alias test-message?

@pull-request-size pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/M PR that changes 30-99 lines. Good size to review. labels Feb 13, 2021
@Kami Kami changed the title [WIP] Add new "st2 action-alias execute-and-format" command for easier ChatOps alias development and testing Add new "st2 action-alias execute-and-format" command for easier ChatOps alias development and testing Feb 13, 2021
@Kami
Copy link
Member Author

Kami commented Feb 13, 2021

Added basic unit test (it's mock based so it's not ideal, but I dont have access nor time for end to end test at this point which would likely be a multi day commitment).

Docs are available at StackStorm/st2docs#1053.

blag
blag previously requested changes Apr 22, 2021
Copy link
Contributor

@blag blag left a comment

Choose a reason for hiding this comment

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

Looks pretty good, but...f-strings!

st2client/st2client/commands/action_alias.py Outdated Show resolved Hide resolved
st2client/st2client/commands/action_alias.py Outdated Show resolved Hide resolved
st2client/st2client/commands/action_alias.py Outdated Show resolved Hide resolved
@blag
Copy link
Contributor

blag commented Apr 22, 2021

Adding end-to-end tests for this should actually be pretty easy and straightforward:

Drop a BATS file into here: https://github.com/StackStorm/st2tests/tree/master/cli

There are plenty of examples in that directory to draw from.

@Kami
Copy link
Member Author

Kami commented Apr 22, 2021

@blag

Adding end-to-end tests for this should actually be pretty easy and straightforward:

Drop a BATS file into here: https://github.com/StackStorm/st2tests/tree/master/cli

There are plenty of examples in that directory to draw from.

Yeah, the problem is that "easy" and "fast" are not always synonyms :D

Adding a test may be easy, but based on my previous experience, the end to end develop + test feedback loop is quite slow and it can easily take multiple hours (and that's an optimistic estimate) to get things developed and tested end to end.

@blag
Copy link
Contributor

blag commented Apr 22, 2021

Since this is for st2client, which is entirely user-facing code, I think end-to-end tests would be good to include for it. If it's worth writing, it's worth writing tests to make sure it doesn't break in the future.

@Kami
Copy link
Member Author

Kami commented May 2, 2021

@blag

end to end tests on PRs have been temporary disabled so I probably won't get to that until they are enabled again to run on each PR / on demand (technically I could manually run it, but that's even slower, especially since whole st2web on cicd is so darn slow).

So for the time being we either need to accept and live just with the unit tests or wait until end to end tests are re-enabled again.

Kami and others added 3 commits May 2, 2021 13:14
@amanda11 amanda11 modified the milestones: 3.5.0, 3.6.0 Jun 3, 2021
@CLAassistant
Copy link

CLAassistant commented Sep 6, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

Just a couple of nits.

Also, what did we decide about E2E tests? Are we running them now that we've got some AWS credits?

I'm guessing this will move to 3.7.0 unless it is really close to done.

st2client/tests/unit/test_action_alias.py Outdated Show resolved Hide resolved
st2client/tests/unit/test_action_alias.py Outdated Show resolved Hide resolved
@arm4b arm4b modified the milestones: 3.6.0, 3.7.0 Oct 13, 2021
@Kami
Copy link
Member Author

Kami commented Nov 11, 2021

@cognifloyd

Re style - as mentioned in my previous comment to blag, I just followed the existing code style in the file - which I think it's the best course of actions since we all have our different opinions on style, but automatically enforcing them (to the extend possible) and being consistent is the most important.

As far as end to end tests go, I don't think those will happen any time soon.

@Kami
Copy link
Member Author

Kami commented Nov 11, 2021

@cognifloyd I synced this branch with latest master and resolved conflicts so this should hopefully merge cleanly now.

Follow code style introduced in #5333
@Kami Kami dismissed blag’s stale review March 27, 2022 11:00

For now I kept the code as it to ensure consistency with the existing code base.

@Kami
Copy link
Member Author

Kami commented Mar 27, 2022

Now that v3.6.0 merge freeze is over, I will go ahead and merge it into master.

@Kami Kami merged commit 35fee1d into master Mar 27, 2022
@Kami Kami deleted the add_new_action_alias_execute_and_format_command branch March 27, 2022 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chatops CLI component:st2client size/L PR that changes 100-499 lines. Requires some effort to review. status:needs changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants