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.

Sign up for GitHub

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

Jump to bottom

[P4Testgen] Remove untested/unreliable P4Testgen features. #4165

Merged
merged 2 commits into from
Sep 19, 2023
Merged

[P4Testgen] Remove untested/unreliable P4Testgen features. #4165

merged 2 commits into from
Sep 19, 2023

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Sep 16, 2023

  • Mark the --track-branches option unreliable until we have thoroughly tested it.
  • Remove the max statement coverage code. It is not functional and is superseded by Greedy Statement Coverage.
  • Replace any mention of statement coverage with node coverage to make it clear we are not only tracking statements.

@fruffy fruffy added the p4tools Topics related to the P4Tools back end label Sep 16, 2023
@fruffy fruffy force-pushed the fruffy/remove_unreliable_code branch from 128ce9e to 1db87a2 Compare September 17, 2023 07:13
@fruffy fruffy marked this pull request as ready for review September 18, 2023 10:49
jafingerhut
jafingerhut approved these changes Sep 18, 2023
View reviewed changes
Copy link
Contributor

@jafingerhut jafingerhut left a comment

Choose a reason for hiding this comment

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

LGTM. I feel a little more confident reviewing code that is unfamiliar to me when it is renaming things and removing code/options, hence the approving review here. I am still no expert in p4testgen internals.

@fruffy
Copy link
Collaborator Author

fruffy commented Sep 19, 2023

Thanks!

@fruffy fruffy merged commit 969f7a5 into main Sep 19, 2023
16 checks passed
@fruffy fruffy deleted the fruffy/remove_unreliable_code branch September 19, 2023 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jafingerhut jafingerhut jafingerhut approved these changes

@jnfoster jnfoster Awaiting requested review from jnfoster

@pkotikal pkotikal Awaiting requested review from pkotikal

Assignees
No one assigned
Labels
p4tools Topics related to the P4Tools back end
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fruffy @jafingerhut