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

Start parsing the chunks file with serde #31

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
switch to stable rust and reformat imports
  • Loading branch information
Swatinem committed Nov 19, 2024
commit 7e40eed8d3d83f2b8df95786767b2104ff9fbde0
3 changes: 2 additions & 1 deletion .github/workflows/bench.yml
Original file line number Diff line number Diff line change
@@ -16,7 +16,8 @@ jobs:
with:
lfs: true

- run: rustup toolchain install nightly --profile minimal --no-self-update
- run: rustup toolchain install stable --profile minimal --no-self-update
- uses: Swatinem/rust-cache@v2
- uses: cargo-bins/cargo-binstall@main
- run: cargo binstall cargo-codspeed

14 changes: 7 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -20,7 +20,8 @@ jobs:
steps:
- uses: actions/checkout@v4

- run: rustup toolchain install nightly --profile minimal --component rustfmt --component clippy --no-self-update
- run: rustup toolchain install stable --profile minimal --component rustfmt --component clippy --no-self-update
- uses: Swatinem/rust-cache@v2

- run: cargo fmt --all -- --check
- run: cargo clippy --all-features --workspace --tests --examples -- -D clippy::all
@@ -46,7 +47,8 @@ jobs:
steps:
- uses: actions/checkout@v4

- run: rustup toolchain install nightly --profile minimal --no-self-update
- run: rustup toolchain install stable --profile minimal --no-self-update
- uses: Swatinem/rust-cache@v2

- run: cargo test --workspace --all-features --doc
- run: cargo doc --workspace --all-features --document-private-items --no-deps
@@ -57,14 +59,12 @@ jobs:
steps:
- uses: actions/checkout@v4

- run: rustup toolchain install nightly --profile minimal --no-self-update
- run: rustup toolchain install stable --profile minimal --no-self-update
- uses: Swatinem/rust-cache@v2
- uses: taiki-e/install-action@cargo-llvm-cov
- uses: taiki-e/install-action@nextest

# 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: cargo llvm-cov nextest --lcov --output-path core.lcov --workspace --all-features --all-targets
- run: mv target/nextest/default/core-test-results.xml .

- uses: actions/setup-python@v5
9 changes: 6 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
@@ -17,17 +17,19 @@ All details (e.g. SQLite schema, code interfaces) subject to breaking changes un
## Developing

Set up your development environment:
- Install the nightly compiler via [rustup](https://rustup.rs/). At time of writing, `codecov-rs` requires the nightly compiler for niceties such as `#[feature(trait_alias)]`.

- To work on the Python bindings, run `source .envrc` (or use `direnv`) to set up a virtual environment. Update development dependencies with `pip install -r python/requirements.dev.txt`
- Install lint hooks with `pip install pre-commit && pre-commit install`.
- Large sample test reports are checked in using [Git LFS](https://git-lfs.com/) in `test_utils/fixtures/**/large` directories (e.g. `test_utils/fixtures/pyreport/large`). Tests and benchmarks may reference them so installing it yourself is recommended.

`codecov-rs` aims to serve as effective documentation for every flavor of every format it supports. To that end, the following are greatly appreciated in submissions:

- Thorough doc comments (`///` / `/**`). For parsers, include snippets that show what inputs look like
- Granular, in-module unit tests
- Integration tests with real-world samples (that are safe to distribute; don't send us data from your private repo)

The `core/examples/` directory contains runnable commands for developers including:

- `parse_pyreport`: converts a given pyreport into a SQLite report
- `sql_to_pyreport`: converts a given SQLite report into a pyreport (report JSON + chunks file)

@@ -53,9 +55,8 @@ New parsers should be optional via Cargo features. Adding them to the default fe
Where possible, parsers should not load their entire input or output into RAM. On the input side, you can avoid that with a _streaming_ parser or by using `memmap2` to map the input file into virtual memory. SQLite makes it straightforward enough to stream outputs to the database.

Coverage formats really run the gamut so there's no one-size-fits-all framework we can use. Some options:

- [`quick_xml`](https://crates.io/crates/quick_xml), a streaming XML parser
- [`winnow`](https://crates.io/crates/winnow), a parser combinator framework (fork of [`nom`](https://crates.io/crates/nom))
- `winnow`'s docs illustrate [how one can write a streaming parser](https://docs.rs/winnow/latest/winnow/_topic/partial/index.html)
Comment on lines -57 to -58
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might not use winnow anymore but it is still an option we might use for other parsers in the future (seems like it would be very clean for something like lcov for example). we also don't use quick_xml but that's still in this list

- [`serde`](https://serde.rs/), a popular serialization/deserialization framework
- `serde`'s docs illustrate [how one can write a streaming parser](https://serde.rs/stream-array.html)

@@ -64,6 +65,7 @@ Non-XML formats lack clean OOTB support for streaming so `codecov-rs` currently
### Testing

Run tests with:

```
# Rust tests
$ cargo test
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be cargo nextest?

@@ -75,6 +77,7 @@ $ pytest
### Benchmarks

Run benchmarks with:

```
$ cargo bench --features testing
```
3 changes: 2 additions & 1 deletion bindings/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub use codecov_rs::error::CodecovError as RsCodecovError;
use pyo3::{exceptions::PyRuntimeError, prelude::*};
use pyo3::exceptions::PyRuntimeError;
use pyo3::prelude::*;

pub struct PyCodecovError(RsCodecovError);

3 changes: 2 additions & 1 deletion bindings/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::{fs::File, path::PathBuf};
use std::fs::File;
use std::path::PathBuf;

use codecov_rs::{parsers, report};
use pyo3::prelude::*;
10 changes: 5 additions & 5 deletions core/benches/pyreport.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use std::collections::HashMap;

use codecov_rs::{
parsers::pyreport::{chunks, report_json},
test_utils::test_report::TestReportBuilder,
};
use codecov_rs::parsers::pyreport::{chunks, report_json};
use codecov_rs::test_utils::test_report::TestReportBuilder;
use criterion::{criterion_group, criterion_main, Criterion};
use test_utils::fixtures::{read_fixture, FixtureFormat::Pyreport, FixtureSize::Large};
use test_utils::fixtures::read_fixture;
use test_utils::fixtures::FixtureFormat::Pyreport;
use test_utils::fixtures::FixtureSize::Large;

criterion_group!(
benches,
8 changes: 6 additions & 2 deletions core/examples/parse_pyreport.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use std::{env, fs::File, path::PathBuf};
use std::env;
use std::fs::File;
use std::path::PathBuf;

use codecov_rs::{error::Result, parsers::pyreport::parse_pyreport, report::SqliteReportBuilder};
use codecov_rs::error::Result;
use codecov_rs::parsers::pyreport::parse_pyreport;
use codecov_rs::report::SqliteReportBuilder;

fn usage_error() -> ! {
println!("Usage:");
10 changes: 5 additions & 5 deletions core/examples/sql_to_pyreport.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use std::{env, fs::File};
use std::env;
use std::fs::File;

use codecov_rs::{
error::Result,
report::{pyreport::ToPyreport, SqliteReport},
};
use codecov_rs::error::Result;
use codecov_rs::report::pyreport::ToPyreport;
use codecov_rs::report::SqliteReport;

fn usage_error() -> ! {
println!("Usage:");
4 changes: 0 additions & 4 deletions core/src/error.rs
Original file line number Diff line number Diff line change
@@ -15,10 +15,6 @@ pub enum CodecovError {
#[error("report builder error: '{0}'")]
ReportBuilderError(String),

// Can't use #[from]
#[error("parser error: '{0}'")]
ParserError(winnow::error::ContextError),

#[error("parser error: '{0}'")]
Json(#[from] serde_json::Error),

2 changes: 0 additions & 2 deletions core/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![feature(trait_alias)]

pub mod report;

pub mod parsers;
27 changes: 14 additions & 13 deletions core/src/parsers/pyreport/chunks.rs
Original file line number Diff line number Diff line change
@@ -34,22 +34,23 @@
//! `report_line_or_empty` parser which wraps this and supports empty lines
//! returns `Ok(())`.

use std::{collections::HashMap, fmt, marker::PhantomData, mem, sync::OnceLock};
use std::collections::HashMap;
use std::marker::PhantomData;
use std::sync::OnceLock;
use std::{fmt, mem};

use memchr::{memchr, memmem};
use serde::{de, de::IgnoredAny, Deserialize};

use super::{report_json::ParsedReportJson, utils};
use crate::{
error::CodecovError,
report::{
pyreport::{
types::{self, CoverageType, MissingBranch, Partial, PyreportCoverage, ReportLine},
CHUNKS_FILE_END_OF_CHUNK, CHUNKS_FILE_HEADER_TERMINATOR,
},
Report, ReportBuilder,
},
use serde::de::IgnoredAny;
use serde::{de, Deserialize};

use super::report_json::ParsedReportJson;
use super::utils;
use crate::error::CodecovError;
use crate::report::pyreport::types::{
self, CoverageType, MissingBranch, Partial, PyreportCoverage, ReportLine,
};
use crate::report::pyreport::{CHUNKS_FILE_END_OF_CHUNK, CHUNKS_FILE_HEADER_TERMINATOR};
use crate::report::{Report, ReportBuilder};

#[derive(PartialEq, Debug)]
pub struct ChunkCtx {
3 changes: 2 additions & 1 deletion core/src/parsers/pyreport/mod.rs
Original file line number Diff line number Diff line change
@@ -2,7 +2,8 @@ use std::fs::File;

use memmap2::Mmap;

use crate::{error::Result, report::SqliteReportBuilder};
use crate::error::Result;
use crate::report::SqliteReportBuilder;

pub mod chunks;
pub mod report_json;
9 changes: 4 additions & 5 deletions core/src/parsers/pyreport/report_json.rs
Original file line number Diff line number Diff line change
@@ -163,13 +163,12 @@

use std::collections::{BTreeMap, HashMap};

use serde::{de::IgnoredAny, Deserialize};
use serde::de::IgnoredAny;
use serde::Deserialize;
use serde_json::Value;

use crate::{
error::CodecovError,
report::{models, Report, ReportBuilder},
};
use crate::error::CodecovError;
use crate::report::{models, Report, ReportBuilder};

#[derive(Debug, Deserialize)]
struct ReportJson {
15 changes: 5 additions & 10 deletions core/src/parsers/pyreport/utils.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
use super::chunks::ParseCtx;
use crate::{
error::Result,
report::{
models,
pyreport::types::{
Complexity, CoverageDatapoint, LineSession, MissingBranch, Partial, PyreportCoverage,
ReportLine,
},
Report, ReportBuilder,
},
use crate::error::Result;
use crate::report::pyreport::types::{
Complexity, CoverageDatapoint, LineSession, MissingBranch, Partial, PyreportCoverage,
ReportLine,
};
use crate::report::{models, Report, ReportBuilder};

fn separate_pyreport_complexity(complexity: &Complexity) -> (Option<i64>, Option<i64>) {
let (covered, total) = match complexity {
7 changes: 3 additions & 4 deletions core/src/report/pyreport/chunks.rs
Original file line number Diff line number Diff line change
@@ -3,10 +3,9 @@ use std::io::Write;
use serde_json::{json, Number as JsonNumber, Value as JsonVal};

use super::{CHUNKS_FILE_END_OF_CHUNK, CHUNKS_FILE_HEADER_TERMINATOR};
use crate::{
error::{CodecovError, Result},
report::{models, sqlite::json_value_from_sql, SqliteReport},
};
use crate::error::{CodecovError, Result};
use crate::report::sqlite::json_value_from_sql;
use crate::report::{models, SqliteReport};

/// To save space, trailing nulls are removed from arrays in `ReportLine`s.
///
6 changes: 2 additions & 4 deletions core/src/report/pyreport/mod.rs
Original file line number Diff line number Diff line change
@@ -256,10 +256,8 @@
* - [`CoverageDatapoint`](https://github.com/codecov/shared/blob/f6c2c3852530192ab0c6b9fd0c0a800c2cbdb16f/shared/reports/types.py#L98)
*/

use std::{
fs::File,
io::{BufWriter, Write},
};
use std::fs::File;
use std::io::{BufWriter, Write};

use super::SqliteReport;
use crate::error::Result;
7 changes: 3 additions & 4 deletions core/src/report/pyreport/report_json.rs
Original file line number Diff line number Diff line change
@@ -2,10 +2,9 @@ use std::io::Write;

use serde_json::{json, Value as JsonVal};

use crate::{
error::Result,
report::{models, sqlite::json_value_from_sql, SqliteReport},
};
use crate::error::Result;
use crate::report::sqlite::json_value_from_sql;
use crate::report::{models, SqliteReport};

/// Coverage percentages are written with 5 decimal places of precision unless
/// they are 0 or 100.
3 changes: 2 additions & 1 deletion core/src/report/sqlite/mod.rs
Original file line number Diff line number Diff line change
@@ -6,7 +6,8 @@
* - Some `ORDER BY` clauses are to make writing test cases simple and may
* not be necessary
*/
use std::{path::PathBuf, sync::LazyLock};
use std::path::PathBuf;
use std::sync::LazyLock;

use include_dir::{include_dir, Dir};
use rusqlite::Connection;
10 changes: 3 additions & 7 deletions core/src/report/sqlite/models.rs
Original file line number Diff line number Diff line change
@@ -556,13 +556,9 @@ mod tests {
use serde_json::json;
use tempfile::TempDir;

use super::{
super::{
super::{Report, ReportBuilder},
SqliteReport, SqliteReportBuilder,
},
*,
};
use super::super::super::{Report, ReportBuilder};
use super::super::{SqliteReport, SqliteReportBuilder};
use super::*;

#[derive(PartialEq, Debug)]
struct TestModel {
12 changes: 6 additions & 6 deletions core/src/report/sqlite/report.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use std::{fmt, path::PathBuf};
use std::fmt;
use std::path::PathBuf;

use rusqlite::{Connection, OptionalExtension};

use super::open_database;
use crate::{
error::Result,
report::{models, Report},
};
use crate::error::Result;
use crate::report::{models, Report};

pub struct SqliteReport {
pub filename: PathBuf,
@@ -182,7 +181,8 @@ mod tests {
use rusqlite_migration::SchemaVersion;
use tempfile::TempDir;

use super::{super::SqliteReportBuilder, *};
use super::super::SqliteReportBuilder;
use super::*;
use crate::report::ReportBuilder;

struct Ctx {
15 changes: 6 additions & 9 deletions core/src/report/sqlite/report_builder.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
use std::{
ops::RangeFrom,
path::{Path, PathBuf},
};
use std::ops::RangeFrom;
use std::path::{Path, PathBuf};

use rand::Rng;
use rusqlite::{Connection, Transaction};

use super::{models::Insertable, open_database, SqliteReport};
use crate::{
error::{CodecovError, Result},
report::{models, ReportBuilder},
};
use super::models::Insertable;
use super::{open_database, SqliteReport};
use crate::error::{CodecovError, Result};
use crate::report::{models, ReportBuilder};

/// Returned by [`SqliteReportBuilder::transaction`]. Contains the actual
/// implementation for most of the [`ReportBuilder`] trait except for `build()`
7 changes: 3 additions & 4 deletions core/src/test_utils/sqlite_report.rs
Original file line number Diff line number Diff line change
@@ -2,10 +2,9 @@ use std::path::PathBuf;

use serde_json::json;

use crate::{
error::Result,
report::{models, sqlite::Insertable, ReportBuilder, SqliteReport, SqliteReportBuilder},
};
use crate::error::Result;
use crate::report::sqlite::Insertable;
use crate::report::{models, ReportBuilder, SqliteReport, SqliteReportBuilder};

pub fn build_sample_report(path: PathBuf) -> Result<SqliteReport> {
let mut builder = SqliteReportBuilder::open(path)?;
Loading
Oops, something went wrong.