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 plus to test suites #1536

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

ciarams87
Copy link
Member

Proposed changes

Problem: As a user of NGF that uses an NGINX+ image of NGF
I want the NGINX+ image to be tested with Gateway API's conformance tests and NGINX's non-functional testing
So that I can feel confident that the NGINX+ version of NGF is just as stable as the OSS version.

Solution:

  • Update the conformance test suite to support running the tests for plus
  • Update the conformance CI pipeline to add plus to the matrix
  • Update the NFR test suite to support running the tests for plus

Testing: Ran the conformance tests and the NFR tests using Plus

Please focus on (optional): The AC includes "The release process is updated to include that non-functional tests must be ran against the NGINX+ image in addition to OSS." but we do not have any section in the release process that covers running the NFR tests (as they are currently covered as separate tasks)

Closes #1331

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

@github-actions github-actions bot added documentation Improvements or additions to documentation tests Pull requests that update tests labels Feb 2, 2024
Copy link
Contributor

@kate-osborn kate-osborn left a comment

Choose a reason for hiding this comment

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

LGTM

@ciarams87 ciarams87 marked this pull request as ready for review February 7, 2024 10:12
@ciarams87 ciarams87 requested a review from a team as a code owner February 7, 2024 10:12
@sjberman
Copy link
Contributor

sjberman commented Feb 7, 2024

There appear to be 2 pending checks now, does the new matrix affect the job reporting?

@ciarams87
Copy link
Member Author

There appear to be 2 pending checks now, does the new matrix affect the job reporting?

@sjberman yes, once this is merged I'll update the required checks in the repository settings. Annoyingly, the required checks are defined by job name, and adding a new element to the matrix changes these job names (e.g. Gateway Conformance Tests (latest) -> Gateway Conformance Tests (latest, nginx)). I couldn't add them before the PR was merged because it'll affect the other PRs.

@ciarams87 ciarams87 merged commit 60f3032 into nginxinc:main Feb 8, 2024
30 checks passed
@ciarams87 ciarams87 deleted the test/add-plus-to-conformance branch February 8, 2024 10:47
@sjberman
Copy link
Contributor

sjberman commented Feb 8, 2024

@ciarams87 I wonder why this is a repo setting and not something that you can set in the pipeline itself...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation tests Pull requests that update tests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

NGINX+ Image Conformance and Non-functional testing
4 participants