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

Run conformance tests nightly on main branch #884

Merged
merged 3 commits into from
Jul 21, 2023

Conversation

sjberman
Copy link
Contributor

In order to ensure we are staying ahead of Gateway API changes, we'll now run conformance tests off of the main branch on a nightly schedule.

Devs can also run from the main branch locally by setting the GW_API_VERSION variable.

Closes #668

  • 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

@sjberman sjberman requested a review from a team as a code owner July 18, 2023 20:33
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jul 18, 2023
@pleshakov
Copy link
Contributor

Hi @sjberman

is it possible to run the Gateway API conformance code from the main branch too? in addition to installing the main branch resources.

Looks like we're not on the same page about the requirements. My interpretation of them is to to run both the updated tests and the resources.

@sjberman
Copy link
Contributor Author

@pleshakov Good call, I didn't even think of that.

@sjberman sjberman force-pushed the tests/nightly-testing branch 4 times, most recently from 1b5cc39 to 03ef5e7 Compare July 20, 2023 16:09
@sjberman
Copy link
Contributor Author

@pleshakov Ready for re-review

@pleshakov
Copy link
Contributor

@sjberman
this looks good!

In the previous message #668, I missed one more requirements from that issue " forks from Gateway API." my apologies :(

basically, this is needed so that it is possible to run the tests from someone PR into the Gateway API repo. Like this one kubernetes-sigs/gateway-api#2179
this way, we can provide feedback (if any) and most importantly, hopefully can catch any non-compliant changes before they are merged to the main branch.

I was able to run tests using that fork by doing:

go mod edit -replace=sigs.k8s.io/gateway-api@v0.7.1=github.com/levikobi/gateway-api@attached-routes-conformance
go mod download
go mod verify
go mod tidy

instead of

	go get -u sigs.k8s.io/gateway-api@main
	go mod tidy

result:

        --- PASS: TestConformance/GatewayWithAttachedRoutes/Gateway_listener_should_not_have_the_route_attached_if_the_route_is_not_allowed_by_the_listener's_protocol (0.02s)
...

if that is a good approach, is it possible to enable it in the Makefile and documentation?

@sjberman
Copy link
Contributor Author

@pleshakov I wonder if I document the steps you took, but don't create amake target for it? Since an argument is required (the path to your fork and branch), doesn't really make sense as a target for make. Could be a simple bash script, which I can write, but it's so simple I wonder if simply documenting how to do this is enough for developers, since I doubt this is a workflow that we'd be performing too often. What do you think?

@pleshakov
Copy link
Contributor

@sjberman

Since an argument is required (the path to your fork and branch), doesn't really make sense as a target for make.

what if the default for replacement is github.com/kubernetes-sigs/gateway-api@main ? otherwise, the developer can specify the fork and branch they need

at the same time

@pleshakov I wonder if I document the steps you took, but don't create amake target for it? Since an argument is required (the path to your fork and branch), doesn't really make sense as a target for make. Could be a simple bash script, which I can write, but it's so simple I wonder if simply documenting how to do this is enough for developers, since I doubt this is a workflow that we'd be performing too often. What do you think?

seems reasonable too 👍

In order to ensure we are staying ahead of Gateway API changes, we'll now run conformance tests off of the main branch on a nightly schedule.

Devs can also run from the main branch locally by setting the GW_API_VERSION variable.
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

👍

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

@sjberman sjberman merged commit c382cba into nginxinc:main Jul 21, 2023
17 checks passed
@sjberman sjberman deleted the tests/nightly-testing branch July 21, 2023 15:48
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
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Nightly automated conformance testing and local conformance testing
3 participants