Skip to content

Conversation

@jinpingh
Copy link
Contributor

Fixes StackStorm/st2#4567 : config_context renders against incorrect pack

Add new action to tests pack for render config context integration test.
This is second part of code changes. Please see StackStorm/st2#4570

…pack

Add new action to `tests` pack for render config context integration test.
This is second part of code changes. Please see StackStorm/st2#4570
@jinpingh jinpingh requested review from bigmstone and m4dcoder March 12, 2019 21:08
Copy link
Contributor

@m4dcoder m4dcoder left a comment

Choose a reason for hiding this comment

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

LGTM. Good Job.

jinpingh added a commit to StackStorm/st2 that referenced this pull request Mar 12, 2019
1. Move tests pack action to https://github.com/StackStorm/st2tests/tree/master/packs/tests
   Add a default value to config.schema.yaml and renamed action `render` to `render_config_context`
   Please see StackStorm/st2tests#150
2. Made change in CHANGELOG.rst
3. Rename copy_examples to copy_test_packs in `tools/launchdev.sh`
4. Handle gracefully when authentication is on for setting up virtualenv.
5. Renamed action `render` to `render_config_context` for unit test dummy_pack_7
@jinpingh jinpingh merged commit e61a1dc into master Mar 12, 2019
@jinpingh jinpingh deleted the issue-4567/config_context_renders branch March 12, 2019 23:10
Kami pushed a commit to StackStorm/st2 that referenced this pull request Mar 14, 2019
1. Move tests pack action to https://github.com/StackStorm/st2tests/tree/master/packs/tests
   Add a default value to config.schema.yaml and renamed action `render` to `render_config_context`
   Please see StackStorm/st2tests#150
2. Made change in CHANGELOG.rst
3. Rename copy_examples to copy_test_packs in `tools/launchdev.sh`
4. Handle gracefully when authentication is on for setting up virtualenv.
5. Renamed action `render` to `render_config_context` for unit test dummy_pack_7
@Kami
Copy link
Member

Kami commented Apr 8, 2019

I noticed this breaks st2-self-check script (it looks like it has been failing since this was merged).

Attempting Test tests.render_config_context...ERROR: 400 Client Error: Bad Request
MESSAGE: Additional properties are not allowed ('token', 'protocol' were unexpected) for url: http://127.0.0.1:9101/v1/executions

All the tests inside st2tests pack are designed to also work with a remote StackStorm instance so all the actions take token, protocol and hostname parameters.

@Kami
Copy link
Member

Kami commented Apr 8, 2019

To fix st2-self-check I propose simply removing this new action from this repo and adding it to StackStorm/st2 repo.

Otherwise if we want to get it to work, it will need more changes. Another option would also be to modify st2-self-check so it only runs tests which name starts with test_.

I assume we ended up with broken st2-self-check because of confusing between st2tests directory / package in StackStorm/st2 repo and StackStorm/st2tests repo.

StackStorm/st2tests is used for end to end tests and also for st2-self-check script. For st2-self-check script and end to end tests to work, those tests need to follow a specific format - notably, they need to take token, protocol, hostname parameters.

/cc @m4dcoder @jinpingh

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.

4 participants