etr / libhttpserver

C++ library for creating an embedded Rest HTTP server (and more)
GNU Lesser General Public License v2.1
882 stars 184 forks source link

Count (unexpected) escaping exceptions as a test failure. #316

Closed bcsgh closed 1 year ago

bcsgh commented 1 year ago

Issue or RFC Endorsed by Maintainers

https://github.com/etr/liblittletest/issues/3

Description of the Change

Make LittleTest count exceptions escaping from any of the set-up, tear-down or test-run (other than asserts) as a test failure.

Alternate Designs

None, considered.

Possible Drawbacks

This could cause existing test to start failing, But (fingers crossed) that's only likely for test that should already be failing.

Verification Process

Release Notes

Make LittleTest count exceptions escaping from any of the set-up, tear-down or test-run (other than asserts) as a test failure.

See also:

bcsgh commented 1 year ago

Bump?

I think the CI errors were already there, the test was already broken, it just wasn't being reported as such.

etr commented 1 year ago

I did not have enough time to investigate this, unfortunately.

It appears to be on the setting of protocol priorities for HTTPS. But I need to find the time to run some local tests to see why it keeps failing,

bcsgh commented 1 year ago

IIRC I came to the same conclusion about where the issue was.

I think I ended up using this patch to debug things. https://github.com/bcsgh/libhttpserver/commit/acbd272b78ec441e62891c27895644c309bf2104


Or I might have patched in this:

+    char errbuf[CURL_ERROR_SIZE];
+    curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, errbuf);
     res = curl_easy_perform(curl);
+    if (res) printf("%s\n", errbuf);
bcsgh commented 1 year ago

Rebasing on your https://github.com/etr/libhttpserver/commit/82ae1f088e9e3369a0b2de8d12cfc126200fd05a seems to resolve those prior issues, but now coverage seems to be broken?

Collecting gcovr
  ERROR: HTTP error 502 while getting https://files.pythonhosted.org/packages/11/e9/4eb38ed44d7485a6343f24db3cfc773362a50f3413d7c11678cac2ec6dec/gcovr-6.0-py2.py3-none-any.whl#sha256=2e52019fdb76c6e327f48c2a2d8555fb5e362570b79cc74c5498804d1ce54a60
(from https://pypi.org/simple/gcovr/) (requires-python:>=3.7)

Given that gcovr has needed >=3.7 for over a year I'm guessing something else has changed?

The offending URL has py2 in it which kinda suggests that switching from pip install to pip3 install here might fix things? (Don't know, I don't have time to look at it this morning.)


EDIT:

On further inspection, the failure is while installing gcovr (a coverage tool) on a nocoverage test run. That shouldn't even be happening.

bcsgh commented 1 year ago

Not sure if it was just re-running the tests (I've not managed to repro that fail) or adding https://github.com/bcsgh/libhttpserver/commit/bb132826accf2f3963f03fd6d10b1d41d1e56dd7, but tests are now clean.

etr commented 1 year ago

I've not managed to repro that fail

You mean the failure on the HTTPS priorities? I suspect that setting a set of priorities that made sense actually helped.

bcsgh commented 1 year ago

What I've failed to repro is this failure of pip install gcovr: https://github.com/etr/libhttpserver/actions/runs/4852163596/jobs/8646809985

etr commented 1 year ago

To be fair, you got a Bad Gateway error (502) there. I suspect it was a transient error from pip.

etr commented 1 year ago

You are right, and my github-actions-fu is weak.

See: https://docs.github.com/en/actions/learn-github-actions/expressions#success

It is implicit anyway, so no need to add it

etr commented 1 year ago

I will likely back port some of your changes into: https://github.com/etr/liblittletest

bcsgh commented 1 year ago

I will likely back port some of your changes into: https://github.com/etr/liblittletest

https://github.com/etr/liblittletest/pull/4/files

(There are a few other deltas between the two files that could be cross merged, but that PR should cover the stuff added in the PR.)