Restructure P4Tools to easily add new modules. #3646
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fruffy
changed the title
fruffy
force-pushed
the
fruffy/remove_p4check
branch
from
e393a29
to
f5e4a24
Compare
fruffy
force-pushed
the
fruffy/remove_p4check
branch
6 times, most recently
from
bceeb92
to
11aa399
Compare
pkotikal
reviewed
Nov 22, 2022
| ${TESTGEN_LIBS} | ||
| ) | ||
|
|
||
| add_p4tools_executable(p4testgen main.cpp) | ||
|
|
||
| target_link_libraries( | ||
| p4testgen |
There was a problem hiding this comment.
Should p4testgen be removed?
pkotikal
reviewed
Nov 22, 2022
|
|
||
| try { | ||
| P4Tools::P4Testgen::Testgen().main(args); | ||
| } catch (const Util::CompilerBug& e) { |
There was a problem hiding this comment.
We should add a util function and replace CompilerBug to ToolsBug, TestgenBug or something (not in this PR).
pkotikal
reviewed
Nov 22, 2022
| @@ -3,7 +3,7 @@ include(${CMAKE_CURRENT_LIST_DIR}/../../../cmake/TestUtils.cmake) | |||
| # Add v1model tests from the p4c submodule | |||
| set(P4TOOLS_BINARY_DIR ${CMAKE_SOURCE_DIR}/build) | |||
| set(P4TESTGEN_DIR ${CMAKE_SOURCE_DIR}/build/testgen) | |||
| set(P4TESTGEN_DRIVER "${P4TOOLS_BINARY_DIR}/p4check testgen") | |||
There was a problem hiding this comment.
So p4check doesn't exist at all now. Any module needs to be invoked via it's name like p4testgen?
There was a problem hiding this comment.
Yes, after this PR p4check does not exist any more. All tests are run with the p4testgen binary.
fruffy
force-pushed
the
fruffy/remove_p4check
branch
from
11aa399
to
6c9def4
Compare
fruffy
force-pushed
the
fruffy/remove_p4check
branch
from
6c9def4
to
aadb401
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR restructures P4Tools such that it is easier to add new modules to the framework. The PR removes the p4check binary and instead adds a binary per module. This means, instead of
p4check testgenone can now callp4testgendirectly.To add a new module, a folder inside
p4tools/moduleswith a correspondingCMakelistsfile needs to be created. The CMake script should pick this up directly.