feat(coverage): caching for coverage
#9366
Conversation
yash-atreya
requested review from
DaniPopes,
klkvr,
mattsse,
grandizzy and
zerosnacks
as code owners
yash-atreya
changed the title
coverage): write coverage artifacts to separate dircoverage): separate dir + caching for coverage artifacts
|
did you test that coverage is collected correctly even after multiple different builds? it doesn't look like we account for different |
|
Update: In the previous implementation, we always recompiled the sources when running However, now that we have enabled caching, we cannot assume that Issue demonstrated by test here: e90a354 Next step to tackle this is to make coverage smarter by accounting for |
* fix: use `SourceIdentifier` in SourceFiles mapping * fix(`coverage`): use `SourceIdentifier` in analysis and report. * fix * nit * nit
There was a problem hiding this comment.
we should get rid of indexing by compiler version which was a workaround before we had build_id, including its usage in ContractId
this also does not correctly handle build_ids when processing hitmaps
when we identify a bytecode, we receipt ArtifactId which contains a build_id we should respect vs just the source_id
yash-atreya
changed the title
coverage): separate dir + caching for coverage artifactscoverage): caching for coverage
yash-atreya
changed the title
coverage): caching for coveragecoverage): caching for coverage
| .and_then(|items| items.get_mut(anchor.item_id)) | ||
| .get_mut(&contract_id.source_id) | ||
| .and_then(|items| { | ||
| let scaled_item_id = anchor.item_id % items.len(); |
There was a problem hiding this comment.
this doesn't look correct. right now items are keyed by the source_id they are from but this tries to get it by the source id of the contract where anchor was found
| /// All item anchors for the codebase, keyed by their contract ID. | ||
| pub anchors: HashMap<ContractId, (Vec<ItemAnchor>, Vec<ItemAnchor>)>, | ||
| /// All the bytecode hits for the codebase. | ||
| pub bytecode_hits: HashMap<ContractId, HitMap>, | ||
| /// The bytecode -> source mappings. | ||
| pub source_maps: HashMap<ContractId, (SourceMap, SourceMap)>, | ||
| /// Anchor Item ID by source ID | ||
| pub item_ids_by_source_id: HashMap<SourceIdentifier, Vec<usize>>, |
| /// All coverage items for the codebase, keyed by the compiler version. | ||
| pub items: HashMap<Version, Vec<CoverageItem>>, | ||
| pub items: HashMap<SourceIdentifier, Vec<CoverageItem>>, |
There was a problem hiding this comment.
this should now be a Vec<CoverageItem> I guess given that we no longer have a version-level separation
crates/forge/bin/cmd/coverage.rs
Outdated
| } | ||
|
|
||
| let anchors = artifacts | ||
| .par_iter() | ||
| .filter(|artifact| sources.sources.contains_key(&artifact.contract_id.source_id)) |
There was a problem hiding this comment.
| .filter(|artifact| sources.sources.contains_key(&artifact.contract_id.source_id)) |
this should not be needed anymore
Motivation
Closes: Separate directory for artifacts generated by coverage.
Closes: Enable caching for coverage
Addresses #8840 (comment).
Somewhat addresses #8904 + #8889 - by enabling cache and avoiding recompilation
Solution
out/coverageandout/coverage/build-info.cache/coverage/