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 e2e testing and improve coverage #1204

Merged
merged 25 commits into from May 6, 2020

Conversation

Laremere
Copy link

@Laremere Laremere commented May 2, 2020

This PR refactors the testing so that the in memory and cluster tests share more logic. The om testing struct is also easier to use. All of the e2e test cases now run in the real cluster, instead of a single one.

Resolves #1188
(I chose to use sleeping without custom time code, because the custom time code got overly complex. Instead all sleeps are reduced to 100ms.)

Resolves #1169

Resolves #1052

Long list of random changes:

  • Have evaluator properly stream results back before finishing. This helps make the tests deterministic.
  • Fixed the backend_service using the previous match as the “matchId” in error response.
  • Improved error messages a bit, including using the proto field names.
  • Fixed properly detecting duplicate match id returned by evaluator.
  • Fixed race where an error from the evaluator would start ending the synchronization cycle. If the - - Synchronize call got the end of cycle before the error, it wouldn’t return the error.
  • Fixed the servers closing http’s listener instead of closing the server (whoops).
  • Close http before closing grpc, as http calls the grpc.
  • Use graceful stop when closing grpc, and Shutdown when closing http server. Both will wait until ongoing RPCs finish.
  • Moved the tests and OM startup code into the same package. This simplifies things (mostly main startup for in cluster), but also there’s not really any reason for them to be separate anyways.
  • Removed minimatch test, which was redundant at this point.
  • Some changes to configuration so that required values can be set to reasonable test values.
  • Unified the values between in cluster and in memory (except address and debug flags.)
  • Rewrote tests in match_test into fetch_matches_test to cover same scenarios.
  • Removed "do everything" type tests in favor of tests which cover specific pieces of logic within OM.
  • Renamed mml (mmlogic) to queryClient in matchfunction pkg.

@Laremere Laremere added this to the v1.0.0 milestone May 2, 2020
@Laremere Laremere requested review from sawagh and yfei1 May 2, 2020 00:49
@yfei1
Copy link
Collaborator

yfei1 commented May 4, 2020

Honestly speaking I am lost in the halfway doing the review... 😵 May need one more day to digest this PR.

Copy link
Collaborator

@yfei1 yfei1 left a comment

Choose a reason for hiding this comment

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

I haven't went through the tests yet, will look into them later today.

@Laremere
Copy link
Author

Laremere commented May 5, 2020

/gcprun

@yfei1
Copy link
Collaborator

yfei1 commented May 5, 2020

/gcbrun

@yfei1
Copy link
Collaborator

yfei1 commented May 5, 2020

Looks like the test failed for some reason. Adding resolver.Get("dns") and resolver.Get("passthrough") back to cluster.go and in_memory.go may help.

internal/testing/e2e/fetch_matches_test.go Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@Laremere Laremere merged commit 8363bc5 into googleforgames:master May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants