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

[P4Testgen] Initialize the testgen targets when invoking the library API #4706

Merged
merged 2 commits into from
Jul 27, 2024

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Jun 4, 2024

This is a problem that was not caught in testing.

The failure isn't caught by the tests because of a nasty side effect of static allocation. gtest initializes the target at some point for another tests and it remains initialized for the tests that should fail.

#4286 is relevant here.

@fruffy fruffy added the p4tools Topics related to the P4Tools back end label Jun 4, 2024
vlstill
vlstill previously approved these changes Jun 5, 2024
@vlstill vlstill dismissed their stale review June 5, 2024 07:33

Dismissing because I've noticed the modified test fails.

@fruffy
Copy link
Collaborator Author

fruffy commented Jul 1, 2024

Depends on #4728.

@fruffy fruffy enabled auto-merge July 17, 2024 13:17
@fruffy fruffy disabled auto-merge July 17, 2024 13:17
@fruffy fruffy enabled auto-merge July 17, 2024 13:18
@fruffy fruffy requested a review from vlstill July 17, 2024 20:39
@fruffy
Copy link
Collaborator Author

fruffy commented Jul 25, 2024

@vlstill This passes now after #4728 has been merged.

@fruffy fruffy requested a review from smolkaj July 26, 2024 22:04
Copy link
Member

@smolkaj smolkaj left a comment

Choose a reason for hiding this comment

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

Thanks!

@fruffy fruffy added this pull request to the merge queue Jul 27, 2024
Merged via the queue into p4lang:main with commit 4c25ea5 Jul 27, 2024
17 checks passed
@fruffy fruffy deleted the fruffy/testgen_lib_fix branch July 27, 2024 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4tools Topics related to the P4Tools back end
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants