gatorgrader icon indicating copy to clipboard operation
gatorgrader copied to clipboard

Feature/execute failing command

Open Yanqiao4396 opened this issue 3 years ago • 3 comments

A new check to ensure the wrong command should return exit code 1 as expected. As long as it crashes and has return code 1 as expected, the related check should pass

What is the current behavior?

This feature is achieved by a new check type: ExecuteFailingCommand with the necessary flag: command. Unite test is passed and the code is proved to be vital in gatorgrade, which is shown in the screenshot below Screen Shot 2022-10-17 at 10 36 47 AM

What is the new behavior if this PR is merged?

Other information

This PR has:
  • [x] Commit messages that are correctly formatted
  • [x] Tests for newly introduced code
  • [x] Docstrings for newly introduced code

This PR is a feature

Developers

@Yanqiao4396 @jnormile @burgess01

Yanqiao4396 avatar Oct 17 '22 17:10 Yanqiao4396

Hello @Yanqiao4396, thanks for creating this PR and sharing details about the feature. Quick questions:

  • Have you done integration testing with this feature by using gatorgrade to invoke it?
  • If you have, then can you show what the configuration file would look like in gatorgrade.yml?
  • If you have, then can you show what would be the output from running this command?
  • If you have not, then do you think that it would be possible to perform some type of integration testing?

Please note that it would be fine if you perform some type of manual integration testing! To be clear, I'm not requesting that you implement and run any type of automated integration tests. Although, that type of test suite is sorely needed!

gkapfham avatar Oct 18 '22 20:10 gkapfham

@gkapfham , you could check the screenshot I attached above if it makes sense to you, it's gatorgrade other than gatorgrader which I was running there. It mimiced the process of invoking gatorgrader with arguments list. So I may think it's integration test to show gatorgrade can call gatorgrader as a dependency with the new check ExecuteFailingCommand. So somehow, I feel I've done automated integration tests there even if I didn't push the test into gatorgrade. I didn't because there is no specific test case for specific check in gatorgrade level. When I said unit test, I may I created the test functions for the source codes I changed in gatorgrader. Sorry for confusion.

Yanqiao4396 avatar Oct 18 '22 21:10 Yanqiao4396

@gkapfham, I locally installed this branch's version of Gatorgrader and locally installed a version of gatorgrade that didn't have the specified version of gatorgrader listed. I used the command shell lab from operating systems in order to check this, as it was easy to simulate passing and failing builds. Due to this I found a slight error with this code, where since I was using sys.exit() and try/catch blocks to handle errors but still throw a 1 exit code at the end weren't passing with a failure. Through further testing, this turns out to be because it wasn't passing the requirement for both a failing build AND an error message, so I changed that to be an or statement, as realistically if either of those happens that means the build failed. Attached are the results of running the new tests I created as well as the code for the tests. If you need anything else to show that our code is now working I would be happy to provide it!

Screen Shot 2022-11-18 at 3 55 44 PM

burgess01 avatar Nov 18 '22 20:11 burgess01