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

refactor: organize targets by service not endpoint #567

Merged
merged 2 commits into from
Aug 15, 2024

Conversation

malandis
Copy link
Contributor

Previously we organized the make targets by endpoint. This isn't
granular enough to test when some services aren't deployed at an
endpoint (eg leaderboards in the cache endpoint).

With this change we test by service. Because of this we simplify the
test organization by not grouping by control vs data.

Previously we organized the make targets by endpoint. This isn't
granular enough to test when some services aren't deployed at an
endpoint (eg leaderboards in the cache endpoint).

With this change we test by service. Because of this we simplify the
test organization by not grouping by control vs data.
@malandis malandis requested review from cprice404 and a team August 14, 2024 23:56
We rename token -> auth and remove references to the control endpoint.
TEST_TARGETS_CONTROL_ENDPOINT := test-dotnet6-control-endpoint test-dotnet-framework-control-endpoint
TEST_TARGETS_TOKEN_ENDPOINT := test-dotnet6-token-endpoint test-dotnet-framework-token-endpoint
TEST_TARGETS_AUTH_SERVICE := test-dotnet6-auth-service test-dotnet-framework-auth-service
TEST_TARGETS_CACHE_SERVICE := test-dotnet6-cache-service test-dotnet-framework-cache-service
Copy link
Contributor

Choose a reason for hiding this comment

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

@malandis I think we still need to keep control plane tests separate since it has a different endpoint, right? sorry if I missed calling this out when you pinged me in slack the other day. lmk if you want to zoom about it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clearing that up. As discussed we can loop back and split this up additionally by endpoint.

Copy link
Contributor

@cprice404 cprice404 left a comment

Choose a reason for hiding this comment

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

fine to merge this and then come back and hit the endpoint stuff

@malandis malandis merged commit 8f9eb8f into main Aug 15, 2024
8 checks passed
@malandis malandis deleted the refactor/makefile-targets-by-service branch August 15, 2024 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants