Skip to content

Conversation

@webknjaz
Copy link
Member

What do these changes do?

This patch sets up the infrastructure for collecting and reporting coverage for C-extensions. It includes updates to packaging, test runner and CI/CD.

Warning

THE PATCH IS INCOMPLETE. THIS IS @webknjaz's PLAYGROUND FOR NOW. DO NOT MERGE!

I've been able to generate coverage reports locally, with a series of manual commands. They still need to be integrated and cleaned up to fit well together.

Are there changes in behavior for the user?

This is a contributor-facing change, allowing them to expose gaps in coverage of C-extensions.

Related issue number

N/A

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

@webknjaz webknjaz self-assigned this Jun 28, 2025
@asvetlov
Copy link
Member

I expect a far from 100% coverage because of CPython API calls error checking.
It is not a stopper but we should life with it; not all error checks are reproducible.
E.g., man calls could return NULL with MemoryError set, but realistically there is no way to write a test for such situation. And so on.

@webknjaz webknjaz force-pushed the maintenance/c-coverage-infra branch 5 times, most recently from 5ea38c2 to 453481f Compare July 1, 2025 01:04
log.info('Copying tracing data into the build directory')
tracing_data_file_in_tmp_dir = self._ext_tracing_file_for(ext)
tracing_data_in_package_dir = self._ext_tracing_data_dir_map[ext.name]
tracing_data_file_in_package_dir = tracing_data_in_package_dir / tracing_data_file_in_tmp_dir

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable tracing_data_file_in_package_dir is not used.
@codspeed-hq
Copy link

codspeed-hq bot commented Jul 1, 2025

CodSpeed Performance Report

Merging #1228 will improve performances by 12.2%

Comparing webknjaz:maintenance/c-coverage-infra (8bea87b) with master (00e3803)

Summary

⚡ 1 improvements
✅ 244 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
test_multidict_getall_str_hit[cs-c] 14.8 ms 13.2 ms +12.2%

@webknjaz webknjaz force-pushed the maintenance/c-coverage-infra branch from 0bc563e to 8bea87b Compare July 7, 2025 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants