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

[move-coverage]: change visibility for uncovered locations #14845

Merged

Conversation

Rqnsom
Copy link
Contributor

@Rqnsom Rqnsom commented Oct 2, 2024

Description

In the move-mutation-test tool, the uncovered_locations from SourceCoverageBuilder are going to be used to generate mutants for specific pieces of source code that hasn't been covered by the unit test coverage.

How Has This Been Tested?

N/A

Key Areas to Review

N/A

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Move Compiler
  • Other (specify)

Checklist

  • I have read and followed the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I identified and added all stakeholders and component owners affected by this change as reviewers
  • I tested both happy and unhappy path of the functionality
  • I have made corresponding changes to the documentation

Copy link

trunk-io bot commented Oct 2, 2024

@vineethk vineethk self-requested a review October 2, 2024 23:45
@@ -27,15 +27,23 @@ use std::{
str::FromStr,
};

/// Function source coverage.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Function source coverage.
/// Source-level coverage information for a function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

#[derive(Clone, Debug, Serialize)]
pub struct FunctionSourceCoverage {
/// Indication for native functions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Indication for native functions.
/// Is this a native function?
/// If so, then `uncovered_locations` is empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

pub fn_is_native: bool,

/// List of uncovered locations in the function.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// List of uncovered locations in the function.
/// List of source locations in the function that were not covered.

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't looked at the code deeply, I am assuming this is what it means from looking here. Please double check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's right! I'm using this in the move-mutation-test a lot these days.
Done

#[derive(Debug, Serialize)]
pub struct SourceCoverageBuilder<'a> {
uncovered_locations: BTreeMap<Identifier, FunctionSourceCoverage>,
/// List of uncovered locations in the source code.
Copy link
Contributor

Choose a reason for hiding this comment

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

Mapping from function name to the source-level uncovered locations for that function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

/// List of uncovered locations in the source code.
pub uncovered_locations: BTreeMap<Identifier, FunctionSourceCoverage>,

/// Source map.
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant comment, remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

In the `move-mutation-test` tool, the `uncovered_locations` from
`SourceCoverageBuilder` are going to be used to generate mutants
for specific pieces of source code that hasn't been covered by
the unit test coverage.
@Rqnsom Rqnsom force-pushed the chore/change-visibility-in-the-coverage branch from 2dbeacf to 9184d05 Compare October 3, 2024 07:33
Copy link

codecov bot commented Oct 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.8%. Comparing base (9271edf) to head (197e35b).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14845   +/-   ##
=======================================
  Coverage    59.8%    59.8%           
=======================================
  Files         853      853           
  Lines      208159   208163    +4     
=======================================
+ Hits       124530   124547   +17     
+ Misses      83629    83616   -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vineethk vineethk enabled auto-merge (squash) October 3, 2024 14:20

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@vineethk vineethk added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Oct 3, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Oct 3, 2024

✅ Forge suite realistic_env_max_load success on 8b1674a26ae5b07b223f9aafc91d2476812ba9c8

two traffics test: inner traffic : committed: 13766.64 txn/s, latency: 2887.79 ms, (p50: 2700 ms, p70: 3000, p90: 3000 ms, p99: 3600 ms), latency samples: 5234380
two traffics test : committed: 100.10 txn/s, latency: 2588.21 ms, (p50: 2400 ms, p70: 2500, p90: 2700 ms, p99: 9600 ms), latency samples: 1700
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.249, avg: 0.220", "QsPosToProposal: max: 0.274, avg: 0.246", "ConsensusProposalToOrdered: max: 0.321, avg: 0.297", "ConsensusOrderedToCommit: max: 0.505, avg: 0.474", "ConsensusProposalToCommit: max: 0.801, avg: 0.771"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.92s no progress at version 1916647 (avg 0.21s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.49s no progress at version 1916645 (avg 7.63s) [limit 15].
Test Ok

Copy link
Contributor

github-actions bot commented Oct 3, 2024

✅ Forge suite compat success on e8a6faf60a98afca8982e0582f929401294bbd33 ==> 8b1674a26ae5b07b223f9aafc91d2476812ba9c8

Compatibility test results for e8a6faf60a98afca8982e0582f929401294bbd33 ==> 8b1674a26ae5b07b223f9aafc91d2476812ba9c8 (PR)
1. Check liveness of validators at old version: e8a6faf60a98afca8982e0582f929401294bbd33
compatibility::simple-validator-upgrade::liveness-check : committed: 12721.69 txn/s, latency: 2663.07 ms, (p50: 1900 ms, p70: 2400, p90: 5400 ms, p99: 11600 ms), latency samples: 456820
2. Upgrading first Validator to new version: 8b1674a26ae5b07b223f9aafc91d2476812ba9c8
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 6144.39 txn/s, latency: 4548.76 ms, (p50: 5200 ms, p70: 5400, p90: 6000 ms, p99: 6300 ms), latency samples: 111680
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 6054.33 txn/s, latency: 5330.32 ms, (p50: 5700 ms, p70: 6000, p90: 7100 ms, p99: 7600 ms), latency samples: 201380
3. Upgrading rest of first batch to new version: 8b1674a26ae5b07b223f9aafc91d2476812ba9c8
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 6644.71 txn/s, latency: 4196.64 ms, (p50: 4500 ms, p70: 4800, p90: 5700 ms, p99: 6100 ms), latency samples: 131220
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6789.77 txn/s, latency: 4706.67 ms, (p50: 4800 ms, p70: 5200, p90: 7100 ms, p99: 7400 ms), latency samples: 227040
4. upgrading second batch to new version: 8b1674a26ae5b07b223f9aafc91d2476812ba9c8
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 9854.25 txn/s, latency: 2662.98 ms, (p50: 2900 ms, p70: 3100, p90: 3500 ms, p99: 4000 ms), latency samples: 183000
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 10795.61 txn/s, latency: 2916.00 ms, (p50: 2900 ms, p70: 3000, p90: 3200 ms, p99: 3400 ms), latency samples: 353060
5. check swarm health
Compatibility test for e8a6faf60a98afca8982e0582f929401294bbd33 ==> 8b1674a26ae5b07b223f9aafc91d2476812ba9c8 passed
Test Ok

Copy link
Contributor

github-actions bot commented Oct 3, 2024

✅ Forge suite framework_upgrade success on e8a6faf60a98afca8982e0582f929401294bbd33 ==> 8b1674a26ae5b07b223f9aafc91d2476812ba9c8

Compatibility test results for e8a6faf60a98afca8982e0582f929401294bbd33 ==> 8b1674a26ae5b07b223f9aafc91d2476812ba9c8 (PR)
Upgrade the nodes to version: 8b1674a26ae5b07b223f9aafc91d2476812ba9c8
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1109.60 txn/s, submitted: 1111.58 txn/s, failed submission: 1.98 txn/s, expired: 1.98 txn/s, latency: 2821.81 ms, (p50: 2700 ms, p70: 3000, p90: 4000 ms, p99: 5700 ms), latency samples: 100740
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1151.02 txn/s, submitted: 1154.08 txn/s, failed submission: 3.05 txn/s, expired: 3.05 txn/s, latency: 2559.24 ms, (p50: 2400 ms, p70: 2700, p90: 4200 ms, p99: 5800 ms), latency samples: 105560
5. check swarm health
Compatibility test for e8a6faf60a98afca8982e0582f929401294bbd33 ==> 8b1674a26ae5b07b223f9aafc91d2476812ba9c8 passed
Upgrade the remaining nodes to version: 8b1674a26ae5b07b223f9aafc91d2476812ba9c8
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1125.79 txn/s, submitted: 1126.92 txn/s, failed submission: 1.13 txn/s, expired: 1.13 txn/s, latency: 2855.33 ms, (p50: 2700 ms, p70: 3000, p90: 4200 ms, p99: 7400 ms), latency samples: 99280
Test Ok

@vineethk vineethk merged commit 68bba58 into aptos-labs:main Oct 3, 2024
126 of 137 checks passed
@Rqnsom Rqnsom deleted the chore/change-visibility-in-the-coverage branch October 4, 2024 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants