Skip to content

Commit

Permalink
Fix lints and broken intra-doc links
Browse files Browse the repository at this point in the history
  • Loading branch information
Swatinem committed Nov 11, 2024
1 parent 413d242 commit ae5e39d
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 55 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ jobs:
- uses: taiki-e/install-action@cargo-llvm-cov
- uses: taiki-e/install-action@nextest

- run: cargo llvm-cov nextest --lcov --output-path core.lcov --workspace --all-features --all-targets
# FIXME(swatinem): We should pass `--all-targets` to also compile and tests benchmarks
# Though currently `divan` does not support all CLI arguments as used by `nextest`,
# and benchmarks are unbearably slow anyway, so its not feasible to run in debug builds.
- run: cargo llvm-cov nextest --lcov --output-path core.lcov --workspace --all-features
- run: mv target/nextest/default/core-test-results.xml .

- uses: actions/setup-python@v5
Expand Down
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
[workspace]

resolver = "2"
members = ["bindings", "core", "test_utils"]
default-members = ["core"]
Expand Down
2 changes: 1 addition & 1 deletion core/benches/pyreport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::HashMap;

use codecov_rs::{
parsers::pyreport::{chunks, report_json},
report::test::{TestReport, TestReportBuilder},
test_utils::test_report::{TestReport, TestReportBuilder},
};
use divan::Bencher;
use test_utils::fixtures::{read_fixture, FixtureFormat::Pyreport, FixtureSize::Large};
Expand Down
2 changes: 1 addition & 1 deletion core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ pub mod parsers;
pub mod error;

#[cfg(any(test, feature = "testing"))]
mod test_utils;
pub mod test_utils;
34 changes: 19 additions & 15 deletions core/src/parsers/pyreport/chunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ use super::{
},
utils,
};
#[cfg(doc)]
use crate::report::models;
use crate::report::{
pyreport::{types::*, CHUNKS_FILE_END_OF_CHUNK, CHUNKS_FILE_HEADER_TERMINATOR},
Report, ReportBuilder,
Expand All @@ -28,7 +30,7 @@ use crate::report::{
#[derive(PartialEq, Debug)]
pub struct ChunkCtx {
/// The index of this chunk in the overall sequence of chunks tells us which
/// [`crate::report::models::SourceFile`] this chunk corresponds to.
/// [`SourceFile`](models::SourceFile) this chunk corresponds to.
pub index: usize,

/// Each line in a chunk corresponds to a line in the source file.
Expand All @@ -44,7 +46,7 @@ pub struct ParseCtx<R: Report, B: ReportBuilder<R>> {

/// Tracks the labels that we've already added to the report. The key is the
/// identifier for the label inside the chunks file and the value is the
/// ID of the [`crate::report::models::Context`] we created for it in
/// ID of the [`Context`](models::Context) we created for it in
/// the output. If a `"labels_index"` key is present in the chunks file
/// header, this is populated all at once and the key is a numeric ID.
/// Otherwise, this is populated as new labels are encountered and the key
Expand All @@ -55,12 +57,12 @@ pub struct ParseCtx<R: Report, B: ReportBuilder<R>> {
pub chunk: ChunkCtx,

/// The output of the report JSON parser includes a map from `chunk_index`
/// to the ID of the [`crate::report::models::SourceFile`] that the
/// to the ID of the [`SourceFile`](models::SourceFile) that the
/// chunk corresponds to.
pub report_json_files: HashMap<usize, i64>,

/// The output of the report JSON parser includes a map from `session_id` to
/// the ID of the [`crate::report::models::Context`] that the session
/// the ID of the [`Context`](models::Context) that the session
/// corresponds to.
pub report_json_sessions: HashMap<usize, i64>,
}
Expand Down Expand Up @@ -121,8 +123,8 @@ pub fn coverage<S: StrStream, R: Report, B: ReportBuilder<R>>(
.parse_next(buf)
}

/// Parses the coverage type described by a `ReportLine`. Beware: this field may
/// be inaccurate.
/// Parses the coverage type described by a [`ReportLine`]. Beware: this field
/// may be inaccurate.
///
/// For example, in a chunks file for a Go project, the "coverage type" field is
/// always `null` when some of the values in the "coverage" field indicate the
Expand All @@ -141,7 +143,8 @@ pub fn coverage_type<S: StrStream, R: Report, B: ReportBuilder<R>>(
.parse_next(buf)
}

/// Parses value of the "complexity" field in a `ReportLine` or `LineSession`.
/// Parses value of the "complexity" field in a [`ReportLine`] or
/// [`LineSession`].
///
/// Examples: `1`, `3`, `[0, 1]`, `[2, 2]`
pub fn complexity<S: StrStream, R: Report, B: ReportBuilder<R>>(
Expand Down Expand Up @@ -218,9 +221,9 @@ where
.parse_next(buf)
}

/// Parses values in the "partials" field of a `LineSession`. These values don't
/// necessarily have to do with partial branch coverage; what they describe is
/// the coverage status of different subspans of a single line.
/// Parses values in the "partials" field of a [`LineSession`]. These values
/// don't necessarily have to do with partial branch coverage; what they
/// describe is the coverage status of different subspans of a single line.
///
/// Examples:
/// - `[null, 10, 0]`: This line was not covered from its start until column 10
Expand Down Expand Up @@ -254,7 +257,7 @@ pub fn partial_spans<S: StrStream, R: Report, B: ReportBuilder<R>>(
}

/// Parses a [`LineSession`]. Each [`LineSession`] corresponds to a
/// [`crate::report::models::CoverageSample`] in the output report.
/// [`CoverageSample`](models::CoverageSample) in the output report.
///
/// A [`ReportLine`] has a [`LineSession`] for each upload ("session") sent to
/// us for a commit. The [`LineSession`] contains the coverage measurements for
Expand Down Expand Up @@ -346,7 +349,7 @@ pub fn label<S: StrStream, R: Report, B: ReportBuilder<R>>(
///
/// Technically `_coverage_type` is optional, but the way it gets serialized
/// when it's missing is identical to the way we serialize
/// [`crate::report::models::CoverageType::Line`] so there's no way to tell
/// [`CoverageType::Line`] so there's no way to tell
/// which it is when deserializing.
pub fn coverage_datapoint<S: StrStream, R: Report, B: ReportBuilder<R>>(
buf: &mut ReportOutputStream<S, R, B>,
Expand All @@ -369,11 +372,12 @@ pub fn coverage_datapoint<S: StrStream, R: Report, B: ReportBuilder<R>>(

/// Parses a [`ReportLine`]. A [`ReportLine`] itself does not correspond to
/// anything in the output, but it's an umbrella that includes all of the data
/// tied to a line/[`CoverageSample`].
/// tied to a line/[`CoverageSample`](models::CoverageSample).
///
/// This parser performs all the writes it can to the output
/// stream and only returns a `ReportLine` for tests. The `report_line_or_empty`
/// parser which wraps this and supports empty lines returns `Ok(())`.
/// stream and only returns a [`ReportLine`] for tests. The
/// `report_line_or_empty` parser which wraps this and supports empty lines
/// returns `Ok(())`.
pub fn report_line<'a, S, R: Report, B: ReportBuilder<R>>(
buf: &mut ReportOutputStream<S, R, B>,
) -> PResult<ReportLine>
Expand Down
3 changes: 2 additions & 1 deletion core/src/parsers/pyreport/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ fn format_pyreport_branch(branch: &MissingBranch) -> (models::BranchFormat, Stri
}

/// Each [`LineSession`] corresponds to a single [`models::CoverageSample`].
/// Each [`CoverageSample`] _may_ (but won't always) have:
/// Each [`CoverageSample`](models::CoverageSample) _may_ (but won't always)
/// have:
/// - multiple related [`models::BranchesData`] records, one for each specific
/// branch path we have data for
/// - a single related [`models::MethodData`] if the `LineSession` is for a
Expand Down
4 changes: 2 additions & 2 deletions core/src/report/pyreport/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
* [`shared/reports/types.py`](https://github.com/codecov/shared/blob/main/shared/reports/types.py),
* and [`shared/utils/sessions.py`](https://github.com/codecov/shared/blob/main/shared/utils/sessions.py).
*
* Parsers that will build a [`SQLiteReport`] from these parts live in
* Parsers that will build a [`SqliteReport`] from these parts live in
* [`crate::parsers::pyreport`] but code that will convert a
* [`SQLiteReport`] back into a Pyreport lives here.
* [`SqliteReport`] back into a Pyreport lives here.
*
* # Report JSON
*
Expand Down
35 changes: 19 additions & 16 deletions core/src/report/pyreport/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use std::collections::HashMap;

pub use super::super::models::CoverageType;
use crate::parsers::json::JsonVal;
#[cfg(doc)]
use crate::report::models;

/// Enum representing the possible values of the "coverage" field in a
/// ReportLine or LineSession object.
Expand Down Expand Up @@ -63,22 +65,22 @@ pub struct Partial {
}

/// Represents the coverage measurements taken for a specific "session". Each
/// `LineSession` will correspond to a
/// [`crate::report::models::CoverageSample`].
/// [`LineSession`] will correspond to a
/// [`CoverageSample`](models::CoverageSample).
///
/// Each upload to our system constitutes a "session" and may be tagged with
/// flags or other context that we want to filter on when viewing report data.
#[derive(Debug, PartialEq)]
pub struct LineSession {
/// This ID indicates which session the measurement was taken in. It can be
/// used as a key in `buf.state.report_json_sessions` to get the ID of a
/// [`crate::report::models::Context`] in order to create a
/// [`crate::report::models::ContextAssoc`].
/// [`Context`](models::Context) in order to create a
/// [`ContextAssoc`](models::ContextAssoc).
pub session_id: usize,

/// The coverage measurement that was taken in this session. The
/// `CoverageType` is "inherited" from the [`ReportLine`] that this
/// `LineSession` is a part of.
/// [`CoverageType`] is "inherited" from the [`ReportLine`] that this
/// [`LineSession`] is a part of.
pub coverage: PyreportCoverage,

/// A list of specific branches/conditions stemming from this line that were
Expand Down Expand Up @@ -106,13 +108,13 @@ pub enum RawLabel {
/// A numeric ID that was assigned to this label. The original label can be
/// accessed in the `"labels_index"` key in the chunks file's header.
/// For our parser's purposes, we can access the ID of the
/// [`crate::report::models::Context`] created for this label in
/// [`Context`](models::Context) created for this label in
/// `buf.state.labels_index`.
LabelId(u32),

/// The name of the label. If we have encountered this label before, it
/// should be in `buf.state.labels_index` pointing at the ID for a
/// [`crate::report::models::Context`]. Otherwise, we should create that
/// [`Context`](models::Context). Otherwise, we should create that
/// `Context` + mapping ourselves.
LabelName(String),
}
Expand All @@ -124,20 +126,20 @@ pub enum RawLabel {
pub struct CoverageDatapoint {
/// This ID indicates which session the measurement was taken in. It can be
/// used as a key in `buf.state.report_json_sessions` to get the ID of a
/// [`crate::report::models::Context`] in order to create a
/// [`crate::report::models::ContextAssoc`].
/// [`Context`](models::Context) in order to create a
/// [`ContextAssoc`](models::ContextAssoc).
pub session_id: u32,

/// A redundant copy of the coverage measurement for a session. We prefer
/// the value from the [`LineSession`].
pub _coverage: PyreportCoverage,

/// A redundant copy of the `CoverageType`. We use the value from the
/// A redundant copy of the [`CoverageType`]. We use the value from the
/// [`ReportLine`].
///
/// Technically this field is optional, but the way we serialize it when
/// it's missing is identical to the way we serialize
/// [`crate::report::models::CoverageType::Line`] so there's
/// [`CoverageType::Line`] so there's
/// no way to tell which it is when deserializing.
pub _coverage_type: Option<CoverageType>,

Expand Down Expand Up @@ -167,7 +169,8 @@ pub struct ReportLine {
pub coverage_type: CoverageType,

/// The list of measurements taken for this line. Each of these corresponds
/// to a [`CoverageSample`] record in a `SqliteReport`.
/// to a [`CoverageSample`](models::CoverageSample) record in a
/// `SqliteReport`.
pub sessions: Vec<LineSession>,

/// Long forgotten field that takes up space.
Expand All @@ -177,9 +180,9 @@ pub struct ReportLine {
/// `sessions`.
pub _complexity: Option<Option<Complexity>>,

/// The list of [`CoverageDatapoint`]s for this line. `CoverageDatapoint` is
/// largely redundant but its `labels` field is the only place where label
/// data is recorded (e.g. which test case was running when this
/// The list of [`CoverageDatapoint`]s for this line. [`CoverageDatapoint`]
/// is largely redundant but its `labels` field is the only place where
/// label data is recorded (e.g. which test case was running when this
/// measurement was collected).
pub datapoints: Option<Option<HashMap<u32, CoverageDatapoint>>>,
}
Expand Down
1 change: 0 additions & 1 deletion core/src/report/sqlite/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,6 @@ mod tests {
env: Some("env".to_string()),
session_type: Some("uploaded".to_string()),
session_extras: Some(json!({})),
..Default::default()
};

model.insert(&ctx.report.conn).unwrap();
Expand Down
30 changes: 16 additions & 14 deletions core/src/report/sqlite/report_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,39 +13,41 @@ use crate::{
};

/// Returned by [`SqliteReportBuilder::transaction`]. Contains the actual
/// implementation for most of the `ReportBuilder` trait except for `build()`
/// implementation for most of the [`ReportBuilder`] trait except for `build()`
/// which is implemented on [`SqliteReportBuilder`]. All
/// `SqliteReportBuilderTx`s created by a `SqliteReportBuilder` must
/// go out of scope before `SqliteReportBuilder::build()` can be called because
/// their `conn` member mutably borrows the SQLite database and prevents
/// `build()` from moving it into a `SqliteReport`.
/// [`SqliteReportBuilderTx`]s created by a [`SqliteReportBuilder`] must
/// go out of scope before [`SqliteReportBuilder::build()`] can be called
/// because their `conn` member mutably borrows the SQLite database and prevents
/// `build()` from moving it into a [`SqliteReport`].
pub struct SqliteReportBuilderTx<'a> {
id_sequence: &'a mut RangeFrom<i64>,

pub filename: &'a Path,
pub conn: Transaction<'a>,
}

impl<'a> SqliteReportBuilderTx<'a> {
impl SqliteReportBuilderTx<'_> {
pub fn rollback(self) -> Result<()> {
Ok(self.conn.rollback()?)
}
}

/// Implementation of the [`ReportBuilder`] trait to build [`SqliteReport`]s.
/// The [`SqliteReportBuilder::transaction`] method returns a
/// [`SqliteReportBuilderTx`], an auxiliary `ReportBuilder` implementation which
/// will run its operations in a transaction that gets committed when the
/// `SqliteReportBuilderTx` goes out of scope. A non-transaction
/// `SqliteReportBuilder`'s `ReportBuilder` functions (except for `build()`)
/// [`SqliteReportBuilderTx`], an auxiliary [`ReportBuilder`] implementation
/// which will run its operations in a transaction that gets committed when the
/// [`SqliteReportBuilderTx`] goes out of scope. A non-transaction
/// [`SqliteReportBuilder`]'s `ReportBuilder` functions (except for `build()`)
/// call `self.transaction()?` for each call and delegate to the
/// `SqliteReportBuilderTx` implementation.
/// [`SqliteReportBuilderTx`] implementation.
pub struct SqliteReportBuilder {
pub filename: PathBuf,
pub conn: Connection,

/// A single sequence is shared for [`CoverageSample`], [`BranchesData`],
/// [`MethodData`], and [`SpanData`].
/// A single sequence is shared for
/// [`CoverageSample`](models::CoverageSample),
/// [`BranchesData`](models::BranchesData),
/// [`MethodData`](models::MethodData), and [`SpanData`](models::SpanData).
id_sequence: RangeFrom<i64>,
}

Expand Down Expand Up @@ -212,7 +214,7 @@ impl ReportBuilder<SqliteReport> for SqliteReportBuilder {
}
}

impl<'a> ReportBuilder<SqliteReport> for SqliteReportBuilderTx<'a> {
impl ReportBuilder<SqliteReport> for SqliteReportBuilderTx<'_> {
fn insert_file(&mut self, path: &str) -> Result<models::SourceFile> {
let model = models::SourceFile::new(path);
model.insert(&self.conn)?;
Expand Down
2 changes: 0 additions & 2 deletions core/src/test_utils/sqlite_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ pub fn build_sample_report(path: PathBuf) -> Result<SqliteReport> {
env: Some("env upload 1".to_string()),
session_type: Some("type upload 1".to_string()),
session_extras: Some(json!({"k1": "v1"})),
..Default::default()
};
// Insert directly, not through report builder, because we don't want a random
// ID
Expand All @@ -46,7 +45,6 @@ pub fn build_sample_report(path: PathBuf) -> Result<SqliteReport> {
env: Some("env upload 2".to_string()),
session_type: Some("type upload 2".to_string()),
session_extras: Some(json!({"k2": "v2"})),
..Default::default()
};
// Insert directly, not through report builder, because we don't want a random
// ID
Expand Down

0 comments on commit ae5e39d

Please sign in to comment.