Skip to content

Commit

Permalink
address Vineeth's comments
Browse files Browse the repository at this point in the history
  • Loading branch information
brmataptos committed Aug 29, 2024
1 parent 59f18cd commit a82f083
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 10 deletions.
2 changes: 1 addition & 1 deletion crates/aptos/src/move_tool/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ fn compile_coverage(
skip_fetch_latest_git_deps: move_options.skip_fetch_latest_git_deps,
compiler_config: CompilerConfig {
known_attributes: extended_checks::get_all_attribute_names().clone(),
skip_attribute_checks: false,
skip_attribute_checks: move_options.skip_attribute_checks,
bytecode_version: fix_bytecode_version(
move_options.bytecode_version,
move_options.language_version,
Expand Down
8 changes: 5 additions & 3 deletions third_party/move/tools/move-cli/src/base/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,11 @@ impl Coverage {
}) => (module, source_map),
_ => panic!("Should all be modules"),
};
let source_coverage = SourceCoverageBuilder::new(module, &coverage_map, source_map);
let t1 = source_coverage.compute_source_coverage(source_path);
t1.output_source_coverage(&mut std::io::stdout(), self.color, self.tag)
let source_coverage_builder =
SourceCoverageBuilder::new(module, &coverage_map, source_map);
let source_coverage = source_coverage_builder.compute_source_coverage(source_path);

Check warning on line 84 in third_party/move/tools/move-cli/src/base/coverage.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/tools/move-cli/src/base/coverage.rs#L82-L84

Added lines #L82 - L84 were not covered by tests
source_coverage
.output_source_coverage(&mut std::io::stdout(), self.color, self.tag)

Check warning on line 86 in third_party/move/tools/move-cli/src/base/coverage.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/tools/move-cli/src/base/coverage.rs#L86

Added line #L86 was not covered by tests
.unwrap();
},
CoverageSummaryOptions::Summary {
Expand Down
12 changes: 6 additions & 6 deletions third_party/move/tools/move-coverage/src/bin/source-coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ struct Args {
/// Colorize output based on coverage
#[clap(long, default_value_t = ColorChoice::Default)]
pub color: ColorChoice,

Check warning on line 47 in third_party/move/tools/move-coverage/src/bin/source-coverage.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/tools/move-coverage/src/bin/source-coverage.rs#L47

Added line #L47 was not covered by tests
/// Provide textual indication of coverage
#[clap(long, default_value_t = Indicator::Explicit)]
pub explicit: TextIndicator,
/// Tag each line with a textual indication of coverage
#[clap(long, default_value_t = TextIndicator::Explicit)]
pub tag: TextIndicator,

Check warning on line 50 in third_party/move/tools/move-coverage/src/bin/source-coverage.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/tools/move-coverage/src/bin/source-coverage.rs#L50

Added line #L50 was not covered by tests
}

fn main() {
Expand Down Expand Up @@ -78,9 +78,9 @@ fn main() {
None => Box::new(io::stdout()),
};

let t1 = source_cov.compute_source_coverage(source_path);
let t2 = t1
.output_source_coverage(&mut coverage_writer, args.color, args.indicator)
let source_coverage = source_cov.compute_source_coverage(source_path);
source_coverage
.output_source_coverage(&mut coverage_writer, args.color, args.tag)

Check warning on line 83 in third_party/move/tools/move-coverage/src/bin/source-coverage.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/tools/move-coverage/src/bin/source-coverage.rs#L81-L83

Added lines #L81 - L83 were not covered by tests
.unwrap();
}

Expand Down
21 changes: 21 additions & 0 deletions third_party/move/tools/move-coverage/src/source_coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,20 @@ pub enum AbstractSegment {
BoundedLeft { start: u32 },
}

/// Option to control use of color escape codes in coverage output
/// to indicate source code coverage. Unless `None`
/// is selected, code which is covered is green, uncovered
/// code is red. By `Default`, color is only shown when
/// output goes to a terminal. If `Always`, then color
/// escapes are included in the output even to a file
/// or other program.
#[derive(ValueEnum, Clone, Debug, Serialize)]
pub enum ColorChoice {
/// Color is never shown
None,
/// Color is shown only on a terminal
Default,
/// Color is always shown
Always,
}

Expand Down Expand Up @@ -78,10 +88,18 @@ impl FromStr for ColorChoice {
}

Check warning on line 88 in third_party/move/tools/move-coverage/src/source_coverage.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/tools/move-coverage/src/source_coverage.rs#L88

Added line #L88 was not covered by tests
}

/// Option to control use of explicit textual indication of lines
/// covered or not in test coverage listings. If `On` or
/// `Explicit` is selected, then lines with missing coverage
/// are tagged with `-`; otherwise, they have `+`.
#[derive(ValueEnum, Clone, Debug, Serialize)]
pub enum TextIndicator {
/// No textual indicator of coverage.
None,
/// Prefix each line with some code missing coverage by `-`;
/// other lines are prefixed with `+`.
Explicit,
/// Same behavior as Explicit.
On,
}

Expand Down Expand Up @@ -383,6 +401,9 @@ fn merge_spans(file_hash: FileHash, cov: FunctionSourceCoverage) -> Vec<Span> {
.filter(|loc| loc.file_hash() == file_hash)

Check warning on line 401 in third_party/move/tools/move-coverage/src/source_coverage.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/tools/move-coverage/src/source_coverage.rs#L401

Added line #L401 was not covered by tests
.map(|loc| Span::new(loc.start(), loc.end()))
.collect();
if covs.is_empty() {
return vec![];
}

Check warning on line 406 in third_party/move/tools/move-coverage/src/source_coverage.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/tools/move-coverage/src/source_coverage.rs#L404-L406

Added lines #L404 - L406 were not covered by tests
covs.sort();

let mut unioned = Vec::new();
Expand Down

0 comments on commit a82f083

Please sign in to comment.