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

tests: improve readability of output #337

Merged
merged 4 commits into from
Jul 25, 2023
Merged

tests: improve readability of output #337

merged 4 commits into from
Jul 25, 2023

Conversation

jsha
Copy link
Collaborator

@jsha jsha commented Jul 13, 2023

Following up on #336 I realized our test output is pretty hard to read.

In this PR I changed client-server.py so it suppresses client output. If the client command fails, it then prints the suppressed output along with the command line and environment. I'd like to also suppress the server output in the same way but that will require a bigger refactoring, since the server is currently started once and shared by multiple client runs. Most likely we'll wind up wanting to start one server and one client in each test case.

I also added LOG and LOG_SIMPLE macros to client.c and server.c. This simplifies a lot of fprintf calls, and also allows us to print the pid with each log line, which will help tell apart separate server invocations in the logs.

The test server now prints its relevant environment variables at startup.

@jsha
Copy link
Collaborator Author

jsha commented Jul 14, 2023

I got another flake in rustls-ffi / Build+test (macos-latest, clang, stable) (pull_request), in the run_mtls_client_crl_tests part of the tests. A snippet below. The "connecting: Connection refused" message comes from a perror call in client.c that I'll add a prefix to.

023-07-13T23:50:30.5458150Z server[2947]: read 246 bytes from socket
2023-07-13T23:50:30.5459520Z server[2947]: do_read returning 0
2023-07-13T23:50:30.5460010Z server[2947]: reading from socket: EAGAIN or EWOULDBLOCK: Resource temporarily unavailable
2023-07-13T23:50:30.5460990Z server[2947]: writing response
2023-07-13T23:50:30.5461560Z server[2947]: negotiated ALPN protocol: 'http/1.1'
2023-07-13T23:50:30.5463810Z server[2947]: closing socket 5
2023-07-13T23:50:30.7149060Z server[2950]: listening on localhost:8443. AUTH_CERT=testdata/minica.pem, AUTH_CRL=testdata/test.crl.pem, VECTORED_IO=(null)
2023-07-13T23:50:30.7435320Z Traceback (most recent call last):
2023-07-13T23:50:30.7436130Z   File "/Users/runner/work/rustls-ffi/rustls-ffi/./tests/client-server.py", line 47, in run_with_maybe_valgrind
2023-07-13T23:50:30.7436660Z client-server.py: Thu Jul 13 23:50:27 2023 - still trying to connect to localhost:8443
2023-07-13T23:50:30.7437160Z client-server.py: Thu Jul 13 23:50:28 2023 - still trying to connect to localhost:8443
2023-07-13T23:50:30.7437740Z client-server.py: Thu Jul 13 23:50:28 2023 - still trying to connect to localhost:8443
2023-07-13T23:50:30.7438240Z client-server.py: Thu Jul 13 23:50:28 2023 - still trying to connect to localhost:8443
2023-07-13T23:50:30.7438690Z client-server.py: Thu Jul 13 23:50:28 2023 - still trying to connect to localhost:8443
2023-07-13T23:50:30.7439130Z client-server.py: Thu Jul 13 23:50:28 2023 - still trying to connect to localhost:8443
2023-07-13T23:50:30.7439550Z client-server.py: detected server is up on localhost:8443
2023-07-13T23:50:30.7440010Z client-server.py: Thu Jul 13 23:50:29 2023 - still trying to connect to localhost:8443
2023-07-13T23:50:30.7440550Z client-server.py: detected server is up on localhost:8443
2023-07-13T23:50:30.7440720Z 
2023-07-13T23:50:30.7440820Z Failed process output:
2023-07-13T23:50:30.7441050Z client[2951]: connecting to localhost:8443
2023-07-13T23:50:30.7441290Z connecting: Connection refused
2023-07-13T23:50:30.7441430Z 
2023-07-13T23:50:30.7441600Z     subprocess.run(args, env=process_env, capture_output=True).check_returncode()
2023-07-13T23:50:30.7442820Z   File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/subprocess.py", line 502, in check_returncode
2023-07-13T23:50:30.7458570Z     raise CalledProcessError(self.returncode, self.args, self.stdout,
2023-07-13T23:50:30.7459410Z subprocess.CalledProcessError: Command '['./target/client', 'localhost', '8443', '/']' returned non-zero exit status 1.
2023-07-13T23:50:30.7459680Z 
2023-07-13T23:50:30.7459900Z The above exception was the direct cause of the following exception:
2023-07-13T23:50:30.7460100Z 
2023-07-13T23:50:30.7460210Z Traceback (most recent call last):
2023-07-13T23:50:30.7460680Z   File "/Users/runner/work/rustls-ffi/rustls-ffi/./tests/client-server.py", line 236, in <module>
2023-07-13T23:50:30.7461270Z     main()
2023-07-13T23:50:30.7462010Z   File "/Users/runner/work/rustls-ffi/rustls-ffi/./tests/client-server.py", line 232, in main
2023-07-13T23:50:30.7462700Z     run_mtls_client_crl_tests(client, valgrind)
2023-07-13T23:50:30.7463430Z   File "/Users/runner/work/rustls-ffi/rustls-ffi/./tests/client-server.py", line 139, in run_mtls_client_crl_tests
2023-07-13T23:50:30.7464470Z     run_with_maybe_valgrind(
2023-07-13T23:50:30.7465200Z   File "/Users/runner/work/rustls-ffi/rustls-ffi/./tests/client-server.py", line 52, in run_with_maybe_valgrind
2023-07-13T23:50:30.7465660Z     raise Failure({"args": args, "env": env}) from e
2023-07-13T23:50:30.7466830Z Failure: {'args': ['./target/client', 'localhost', '8443', '/'], 'env': {'CA_FILE': 'testdata/minica.pem', 'AUTH_CERT': 'testdata/example.com/cert.pem', 'AUTH_KEY': 'testdata/example.com/key.pem'}}
2023-07-13T23:50:30.7535730Z make: *** [test] Error 1

Not sure why the client would get a connection refused when client-server.py is getting a successful connection. Also interesting that this happened on macos-latest, clang, stable like the last flake. 🤔

Copy link
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

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

Thank you! This will be a nice improvement. My small comments aren't blocking if you want to ignore them.

tests/client-server.py Outdated Show resolved Hide resolved
tests/client-server.py Outdated Show resolved Hide resolved
@ctz
Copy link
Member

ctz commented Jul 17, 2023

Not sure why the client would get a connection refused when client-server.py is getting a successful connection. Also interesting that this happened on macos-latest, clang, stable like the last flake.

it was a while back that rustls had a test suite that relied on opening and using lots of localhost ports, but back then macos was by far the most mysterious. i'm not sure if it lacks working network namespacing or what. eventually i got to an acceptable place by choosing a different random port for each test, and checking it was free (by attempting a TCP connection) before running the test on it.

jsha added 4 commits July 17, 2023 20:07
 - Suppress output from run_with_maybe_valgrind until and unless
   there is an error.
 - If there is an error, print the environment along with the command.
 - In wait_tcp_port:
   - Don't print an error for the first connect.
   - Shorten the connect cycle times.
   - Make it clear that messages come from client-server.py
- simplify some of the repeated calls to `fprintf`
- add a pid to each log line to make it easier to distinguish different
  invocations
- print out relevant env variables on startup
And start using it in client.c.
Copy link
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

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

LGTM. I threw up a branch to fix the new nightly clippy warnings that failed a couple CI tasks: #339

@jsha jsha merged commit bd39739 into main Jul 25, 2023
40 of 42 checks passed
@jsha jsha deleted the jsha/better-test-output branch July 25, 2023 23:21
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.

None yet

3 participants