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

Ensure failed conformance tests are caught in pipeline & fix local make command #1203

Merged
merged 3 commits into from
Nov 6, 2023

Conversation

ciarams87
Copy link
Member

@ciarams87 ciarams87 commented Oct 26, 2023

Proposed changes

Problem: The variables in the make target for running conformance tests are not being properly evaluated in the pipeline. This means we're missing failed conformance tests in the pipeline at the moment because we always return green even if there was a conformance test failure.

Solution: This PR fixes that by checking if there was a failure in the test run and exiting the CI job with the exit code 2 if there is a failure. Also fixes the case where the target make variables are not being set correctly when the make run-conformance-tests are being ran locally.

Testing: Tested locally and in #1169

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@ciarams87 ciarams87 requested a review from a team as a code owner October 26, 2023 15:23
@github-actions github-actions bot added the chore Pull requests for routine tasks label Oct 26, 2023
@sjberman sjberman self-requested a review October 26, 2023 15:52
@sjberman
Copy link
Contributor

Approved before I saw the failure..so holding off.

@bjee19
Copy link
Contributor

bjee19 commented Oct 26, 2023

So to make sure I'm understanding this correctly, we want the conformance test to be failing as that is how we know we are correctly catching this case in the pipeline? What does "Fail conf tests in pipeline" mean in the title?

@ciarams87
Copy link
Member Author

So to make sure I'm understanding this correctly, we want the conformance test to be failing as that is how we know we are correctly catching this case in the pipeline? What does "Fail conf tests in pipeline" mean in the title?

@bjee19 We're missing failed conformance tests in the pipeline at the moment because we always return green even if there was a conformance test failure. This PR fixes that by checking if there was a failure in the test run and exiting the CI job with the exit code 2 if there is a failure. I'll update the description to be more clear 😄

@ciarams87 ciarams87 changed the title Fail conf tests in pipeline & fix local make command Ensure failed conformance test are caught in pipeline & fix local make command Nov 6, 2023
@ciarams87 ciarams87 changed the title Ensure failed conformance test are caught in pipeline & fix local make command Ensure failed conformance tests are caught in pipeline & fix local make command Nov 6, 2023
@ciarams87 ciarams87 merged commit 60b562b into nginxinc:main Nov 6, 2023
23 checks passed
@ciarams87 ciarams87 deleted the chore/fix-fail-conf-pipeline branch November 6, 2023 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Pull requests for routine tasks
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants