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

Any plans to add Warning/Info status? #191

Closed
vrbcntrl opened this issue Dec 12, 2019 · 28 comments
Closed

Any plans to add Warning/Info status? #191

vrbcntrl opened this issue Dec 12, 2019 · 28 comments
Labels
enhancement waiting for confirmation Workaround/Fix applied, waiting for confirmation

Comments

@vrbcntrl
Copy link
Contributor

Feature Request

Hi eerkunt,

I hope everything is going fine at your end.

Feature description :
do you have any plans to add the test status : Warning or Info?
currently when a test is executed it either Pass or FAIL, however I am wondering if you have any plans to add the status like Warning/Info for cases where it is not necessary to FAIL a test, but return Warnings/Info messages to the consumer?

Thanks in advance!

@eerkunt
Copy link
Member

eerkunt commented Dec 16, 2019

Hi @vrbcntrl,

Could you please explain the use case for this ? Currently we have 2 different messages, which one of them can be seen as INFO a bit. They are ERROR and SKIPPED messages, where we also give some information on SKIPPED messages.

As long as there is a valid use case aligned with the tool's vision, anything is possible. :)

@vrbcntrl
Copy link
Contributor Author

Hi @eerkunt ,

my use case is like this ... when a condition is not met, the test should not fail, i want the test status to be returned as a warning instead of FAILURE.

currently my CICD pipeline will break if there are any test FAILURES, so I don't want to break the pipeline when my test is of type WARNING not PASS|FAILURE type.

I hope that will give some idea what I am trying to achieve.

Thanks.

@eerkunt
Copy link
Member

eerkunt commented Dec 18, 2019

Ah I see, you want to just run terraform-compliance, but not failing. Well, then I think you can use “—wip” argument ?

You can also create tags within your tests and mark those ones (like “test_should_always_fail”) and run them with —tags argument, just to structure it better.

I hope I understood your case correctly.

Please let me know if this wont solve your case

@vrbcntrl
Copy link
Contributor Author

thanks for the understanding.
I have never used "--wip" argument, could you please provide some documentation on this command or usage example? thanks in advance!

@eerkunt
Copy link
Member

eerkunt commented Dec 18, 2019

It is a radish parameter which can be used with terraform-compliance. Couldn't find an exact documentation about that though :(

Whole functional end-to-end tests are running like that, have a look on https://github.com/eerkunt/terraform-compliance/tree/master/tests/functional

It may be a bit hard to understand but it is incorporated in https://github.com/eerkunt/terraform-compliance/blob/9d8ecb55e3fd6fcd15c463cb78f1c2f206918011/tests/functional/run_functional_tests.py#L39

@vrbcntrl
Copy link
Contributor Author

I just found this link : https://behave.readthedocs.io/en/latest/behave.html#cmdoption-w
looks like this option is used to run scenarios tagged with “wip”, however my requirement is that , run all tests (with out tags) and in some case like I mentioned above, do not FAIL the test, but throw a warning message/ status so that developer could go through the warnings and would fix it if he wanted to , so this way the pipeline will not break for WARNINGS.

@eerkunt
Copy link
Member

eerkunt commented Dec 18, 2019

when you use --wip without tags, ALL failure tests will pass. You won't need any tags

@vrbcntrl
Copy link
Contributor Author

hmmm...let me actually try to use this --wip option and see if it satisfies my requirement...i'll keep you posted.

@eerkunt
Copy link
Member

eerkunt commented Dec 20, 2019

Hi @vrbcntrl ,

Did it work for you ?

@vrbcntrl
Copy link
Contributor Author

Hi @eerkunt ,

Sorry for the delay in my response.
I actually tested my tests with--wipcommand, below are my observations.

my feature file with rule

Feature: Test for EC2 
  Scenario: Rule for EC2 to test the --wip command
    Given I have aws_instance defined
    Then monitoring is be enabled

Test results with out the --wip command

image

and the EXIT_CODE is 1 in this case

Test results with the --wip command

image

and the EXIT_CODE is 0 in this case

in both cases the test FAILED and thrown a FAILURE message , however I understand when the --wip command used the EXIT_CODE returned is 0 instead of 1 when the test FAILED.

However, my requirement is that when the test is run, I want terraform-compliance to return WARNING/INFO message something like below even though the condition is not met

Example WARNING/INFO message:

 @warning
 Scenario: Rule for EC2 to test the --wip command
        Given I have aws_instance defined
        Then monitoring is be enabled
          WARNING/INFO: Resource aws_instance.web does not have monitoring property enabled (monitoring=None).

1 features (0 passed, 1 warning)
1 scenarios (0 passed, 1 warning)
2 steps (1 passed, 1 warning)

do you think this is possible?

Thanks in advance!

@eerkunt
Copy link
Member

eerkunt commented Dec 20, 2019

I think this is possible but requires #170 to be completed first, since listing all failures would be logical if we are not going to fail the tests.

Will also do this on Xmas break.

@vrbcntrl
Copy link
Contributor Author

Awesome!!!
Thank you very much @eerkunt for all your efforts.

Happy holidays :)

@eerkunt eerkunt mentioned this issue Dec 28, 2019
18 tasks
@eerkunt
Copy link
Member

eerkunt commented Jan 3, 2020

Added -n/--no-failure option where exit code is always 0 even there is a failure - which fulfils this requirement as I understand.

This issue will be considered as fixed after #196 is merged.

@eerkunt
Copy link
Member

eerkunt commented Feb 1, 2020

Hi @vrbcntrl,

Have a try with 1.1.0 or 1.1.1 and let me know if this will fit your use case.

Thanks! 🎉

@eerkunt eerkunt added the waiting for confirmation Workaround/Fix applied, waiting for confirmation label Feb 1, 2020
@vrbcntrl
Copy link
Contributor Author

vrbcntrl commented Feb 3, 2020

Hi @eerkunt ,

Thanks for the update. I have tested my use case with v 1.1.3 and below is my output

image

so, like I mentioned in the screen shot, I want the status to be displayed as "Warning" instead of "Failure". please let me know if that is feasible?

may be if we mark the rule/test with a tag something like "@warning", would it be possible to return the Warning status for the rules that are marked with @warning tags? please let me know.

Thanks in advance!

@eerkunt
Copy link
Member

eerkunt commented Feb 3, 2020

Hi @vrbcntrl,

I thought, the main use case you have asked is to continue processing and show all failures that exists and do not fail the whole execution.

Currently it runs exactly like that, but as I understand instead of Failure you would like to use Warning which sounds really cosmetic. Can you elaborate the reason behind that ?

I also saw other problems, it looks like terminal colours and some new lines gone mad in Windows Systems. :(

By the way, yes technically it is possible to change Failure to something that you want unless -q switch is used. Just trying to understand the reasoning.

@eerkunt
Copy link
Member

eerkunt commented Feb 3, 2020

Released 1.1.4 please have a try with that. Still can't understand the use case though :) It will be done via an environment variable, described in here.

@vrbcntrl
Copy link
Contributor Author

vrbcntrl commented Feb 3, 2020

Thanks @eerkunt for the quick update. I have tried with below 2 rules on 1.1.14

@Warning
  Scenario: Rule for EC2 Warning
    Given I have aws_instance defined
    Then monitoring is be enabled
  
 
  Scenario: Rule for EC2 Failure
    Given I have aws_instance defined
    Then monitoring is be enabled 

and I got the below output for a rule with @warning tag and with out the @warning tag rule

image

However, my requirement is to throw a warning message for rules that are marked with @warning tags.

To answer your question on the reasoning:

  • I want the tool to return exit 0 when there are only Warnings, but No Failures so that my Pipeline will not breaks

  • If a rule is marked as @warning, then return only Warning message instead of Failure, so that when we share the report with Dev teams, they fix the Failure cases and we leave it up to them whether to fix the Warnings or not

I hope I am able to explain my use cases better this time :) let me know if you have additional questions.

@vrbcntrl
Copy link
Contributor Author

vrbcntrl commented Feb 3, 2020

forgot to mention that I had created this env variable on my machine before running the above test

Variable : TFC_ERROR
Value: Warning

@eerkunt
Copy link
Member

eerkunt commented Feb 3, 2020

Hi @vrbcntrl,

There was no @warning tag on this update. It was just changing the Failure to Warning as you described in here. Additionaly, I recommend to read new parameters about returning exit code 0. If you don't use -n it won't return you 0 exit code.

It looks like, you actually wanted to tag the Scenarios for fail and no-fail conditions. So on the same run, you want to have conditional checks either a Scenario will fail or not.

I think we can do it either via preconditions (and a new step) or tags, I will have a look.

@vrbcntrl
Copy link
Contributor Author

vrbcntrl commented Feb 3, 2020

Thank you @eerkunt , I appreciate all your efforts on this.

@eerkunt
Copy link
Member

eerkunt commented Feb 3, 2020

I think this time I did what you have asked. Please have a look here. 1.1.5 is coming in few.

@eerkunt
Copy link
Member

eerkunt commented Feb 3, 2020

Yep, 1.1.5 is out, have a try! Please read the doc before you use :)

@vrbcntrl
Copy link
Contributor Author

vrbcntrl commented Feb 3, 2020

its working as expected with v 1.1.15, thanks you very much!

image

another cosmetic change request :)
Would it be possible to change the color for nofail message (may be orange or yellow, but not red)? its not mandatory...but its like nice to have feature I think :)

@eerkunt
Copy link
Member

eerkunt commented Feb 3, 2020

AFAIK, that is not possible :) That is internal to radish. I will have a look though.

@vrbcntrl
Copy link
Contributor Author

vrbcntrl commented Feb 3, 2020

Thank you @eerkunt

@eerkunt
Copy link
Member

eerkunt commented Feb 4, 2020

Just had a look there might be a way but it will make things quite dirty. Due to changes in 1.1.0 that doesn't supported by radish-bdd 0.13.1 it is already dirty enough :) I will pass this cosmetic change for now and will have a look when we have radish-bdd 1.1.0 integrated into terraform-compliance.

Closing the issue, please let me know if you need any further assistance.

@eerkunt eerkunt closed this as completed Feb 4, 2020
@ghost
Copy link

ghost commented Feb 4, 2020

This issue's conversation is now locked. If you want to continue this discussion please open a new issue.

@ghost ghost locked and limited conversation to collaborators Feb 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement waiting for confirmation Workaround/Fix applied, waiting for confirmation
Projects
None yet
Development

No branches or pull requests

2 participants