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

proxy_serve_stale: Test updates #8928

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

bneradt
Copy link
Contributor

@bneradt bneradt commented Jun 27, 2022

This updates the proxy_serve_stale test to run more quickly (about 15
seconds instead of about 2 minutes). While doing so, the test is also
converted to use Proxy Verifier.


Other than adjusting the timing this is not a functional change. The same transactions are performed to exercise serving the stale entries.

@bneradt bneradt added the AuTest label Jun 27, 2022
@bneradt bneradt added this to the 10.0.0 milestone Jun 27, 2022
@bneradt bneradt requested a review from randall June 27, 2022 21:39
@bneradt bneradt self-assigned this Jun 27, 2022
@bneradt
Copy link
Contributor Author

bneradt commented Jun 27, 2022

@serrislew: can you please review this?

@serrislew
Copy link
Contributor

Looks good to me. Is this the standard of how autests should be created now?

This updates the proxy_serve_stale test to run more quickly (about 15
seconds instead of about 2 minutes). While doing so, the test is also
converted to use Proxy Verifier.
@bneradt bneradt force-pushed the proxy_serve_stale_timing_tweaks branch from 781d279 to e746e23 Compare June 28, 2022 15:17
@bneradt
Copy link
Contributor Author

bneradt commented Jun 28, 2022

Looks good to me.

Thank you for the review, @serrislew.

Is this the standard of how autests should be created now?

Standard is a strong word. In case it's helpful to share, here are a couple of my observations with the AuTests:

Test Class Organization

I noticed when reviewing @masaori335's PRs that when he adds AuTests he structures them around a test class with helper methods that define each component of the test (setup_traffic_server, setup_server, run_the_cases, etc.). I find this a very helpful way to organize the test for readability. So I picked this up from him and generally follow his example.

Proxy Verifier

Proxy Verifier has some advantages over curl:

  • It has an expressive traffic verification mechanism that is inlined with the traffic description (the replay files). This is a more purpose built and explicit method for verifying the traffic than using gold files against curl output. Gold files are fine in many contexts (for verifying log output, for instance), but the gold matching mechanism is a pretty generic way to verify output in comparison to being able to verify the characteristics of a header with various directives (equal, contains, present, case sensitive or insensitive, etc), or a response code, or the HTTP body, or any combination of the three.
  • Along these lines, Proxy Verifier test failures in AuTest will print out the specific Verifier error message indicating what went wrong. For example, an error message might say, [ERROR]: HTTP/1 Status Violation: expected 200 got 201, key: push. This is usually a more precise and readable AuTest failure than a gold file diff output.
  • Helpful for this test, it also has delay mechanisms that can be expressed in the replay file.
  • With curl, if a server is needed, the client (curl) and the server (usually microserver) have to be configured independently of each other. With Proxy Verifier, the same replay file configures both the client and the server.
  • When using multiple curl calls in AuTest, they are often separated by independent TestRun objects. (That wasn't the case with this test, but often separate runs are helpful for verifying each curl result independently.) AuTest has some overhead when processing the processes of each test run which slows down the test. When all the transactions are specified in a single TestRun invocation of Proxy Verifier client and server, this can greatly speed up the test.

For these reasons, I prefer using Proxy Verifier over curl in many cases. But I'm biased since I maintain the Proxy Verifier project (@SolidWallOfCode wrote the initial version of Proxy Verifier, I now maintain it). That said, I still do sometimes use curl when all that is needed is a simple request to exercise ATS when no server is needed. In those cases, creating a replay file is verbose in comparison to a simple curl command.


That guides me when I work with AuTests. But I won't block PRs that don't follow these practices.

@bneradt bneradt merged commit 952bf25 into apache:master Jun 28, 2022
@bneradt bneradt deleted the proxy_serve_stale_timing_tweaks branch June 28, 2022 16:35
zwoop pushed a commit that referenced this pull request Jul 6, 2022
This updates the proxy_serve_stale test to run more quickly (about 15
seconds instead of about 2 minutes). While doing so, the test is also
converted to use Proxy Verifier.

(cherry picked from commit 952bf25)
@zwoop
Copy link
Contributor

zwoop commented Jul 6, 2022

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.0 Jul 6, 2022
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Feb 21, 2023
This updates the proxy_serve_stale test to run more quickly (about 15
seconds instead of about 2 minutes). While doing so, the test is also
converted to use Proxy Verifier.

(cherry picked from commit 952bf25)
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Feb 21, 2023
* asf/9.2.x:
  Updated ChangeLog
  trim white spaces before and after the equal sign (apache#8638)
  Fixes compacting spaces in S3 auth plugin (apache#8579)
  Fixed issues when compiling with -Og (apache#8665)
  Update lua plugin examples (apache#8646)
  Additional helper functions for request transform (apache#8631)
  Move ChangeLog and README.md to the lib/fastlz/ (apache#8629)
  Remove unused functions/definitions from ink_defs (apache#8714)
  Stop ATS when a global lua script fails to load (apache#8671)
  Change DNS retries to be a static (requires restart) config value (apache#8724)
  add log format for whether origin TLS connection resumed an existing TLS session (apache#8745)
  Update location for core rule set in modsecurity example (apache#8924)
  Remove unnecessary use of a memory arena when logging. (apache#8925)
  Add docs for remap_stats plugin (apache#8927)
  Allows errors from plugin initialization to bubble up (apache#8926)
  proxy_serve_stale: Test updates (apache#8928)
  Make clang-format not modify ink_autoconf.h.in and ink_autoconf.h (apache#8935)
  Fix clang-format installation with multiple threads (apache#8931)
  Add nullptr check of HTTPInfo (apache#8937)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants