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(*) opt-in Valgrind test cases #447

Closed
wants to merge 3 commits into from
Closed

Conversation

thibaultcha
Copy link
Member

@thibaultcha thibaultcha commented Nov 16, 2023

Reworking the way we execute Valgrind checks. ATM, mostly tests on run time for different Wasm runtimes and splitting Valgrind workloads.

The goal of this work will be to make Valgrind tests opt-in: a select set of tests from the whole suite will be enabled for Valgrind testing, therefore avoiding duplicated Valgrind tests (or multiple functional tests with not difference in memory handling), and avoid having to filter-out hundreds of tests only to leave a few enabled ones.

@thibaultcha thibaultcha added the pr/work-in-progress PR: Work in progress label Nov 16, 2023
Copy link

codecov bot commented Nov 16, 2023

Codecov Report

Merging #447 (e9015ff) into main (ecd7896) will increase coverage by 0.00492%.
Report is 1 commits behind head on main.
The diff coverage is 98.36066%.

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##                main        #447         +/-   ##
===================================================
+ Coverage   90.18238%   90.18730%   +0.00491%     
===================================================
  Files             46          46                 
  Lines           8444        8489         +45     
===================================================
+ Hits            7615        7656         +41     
- Misses           829         833          +4     
Files Coverage Δ
src/common/proxy_wasm/ngx_proxy_wasm.c 92.95213% <100.00000%> (+0.02822%) ⬆️
src/common/proxy_wasm/ngx_proxy_wasm_host.c 93.96552% <100.00000%> (+0.63218%) ⬆️
src/http/proxy_wasm/ngx_http_proxy_wasm_dispatch.c 90.67797% <66.66667%> (-1.08339%) ⬇️

... and 2 files with indirect coverage changes

Fix a Valgrind memory leak and an unrelated suppression.

Dispatch resume invoking on_request_headers for subsequent filters may
yield themselves (NGX_AGAIN). In this case, also ensure the dispatch
call is destroyed.

Fix the suppression for headers-more-nginx-module which can happen with
other stacktraces on builtin headers (e.g. Content-Length).
@thibaultcha thibaultcha force-pushed the chore/valgrind-split branch 5 times, most recently from 4841398 to 9cb3518 Compare November 17, 2023 06:07
@thibaultcha thibaultcha changed the title [CI] Valgrind tests tests(*) opt-in Valgrind test cases Nov 29, 2023
@thibaultcha thibaultcha deleted the chore/valgrind-split branch November 29, 2023 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/work-in-progress PR: Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant