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] Allow building p4testgen without BMV2 target #4109

Merged
merged 3 commits into from
Aug 15, 2023

Conversation

vlstill
Copy link
Contributor

@vlstill vlstill commented Aug 14, 2023

  • move the tests that use BMV2 from the global testgen test suite to BMV2 folder so they are only enabled if BVM2 is
  • fix a cmake problem where including only targets that did not define include directories prevented configuration

Note that this in order to properly test the core of testgen you still need BMV2 testgen target enabled.

fixes #4107

@vlstill
Copy link
Contributor Author

vlstill commented Aug 14, 2023

These are the remaining testgen tests that run with BMV2 disabled:

[==========] Running 14 tests from 2 test suites.
[----------] Global test environment set-up.
[----------] 4 tests from FormatTest
[ RUN      ] FormatTest.Format01
[       OK ] FormatTest.Format01 (0 ms)
[ RUN      ] FormatTest.Format02
[       OK ] FormatTest.Format02 (0 ms)
[ RUN      ] FormatTest.Format03
[       OK ] FormatTest.Format03 (0 ms)
[ RUN      ] FormatTest.Format04
[       OK ] FormatTest.Format04 (0 ms)
[----------] 4 tests from FormatTest (0 ms total)

[----------] 10 tests from TaintTest
[ RUN      ] TaintTest.Taint01
[       OK ] TaintTest.Taint01 (0 ms)
[ RUN      ] TaintTest.Taint02
[       OK ] TaintTest.Taint02 (0 ms)
[ RUN      ] TaintTest.Taint03
[       OK ] TaintTest.Taint03 (0 ms)
[ RUN      ] TaintTest.Taint04
[       OK ] TaintTest.Taint04 (0 ms)
[ RUN      ] TaintTest.Taint05
[       OK ] TaintTest.Taint05 (0 ms)
[ RUN      ] TaintTest.Taint06
[       OK ] TaintTest.Taint06 (0 ms)
[ RUN      ] TaintTest.Taint07
[       OK ] TaintTest.Taint07 (0 ms)
[ RUN      ] TaintTest.Taint08
[       OK ] TaintTest.Taint08 (0 ms)
[ RUN      ] TaintTest.Taint09
[       OK ] TaintTest.Taint09 (0 ms)
[ RUN      ] TaintTest.Taint10
[       OK ] TaintTest.Taint10 (0 ms)
[----------] 10 tests from TaintTest (0 ms total)

[----------] Global test environment tear-down
[==========] 14 tests from 2 test suites ran. (0 ms total)
[  PASSED  ] 14 tests.

@vlstill vlstill changed the title [P4Testgen][WIP] Allow building p4testgen without BMV2 target [P4Testgen] Allow building p4testgen without BMV2 target Aug 14, 2023
@vlstill vlstill marked this pull request as ready for review August 14, 2023 17:26
@@ -52,15 +52,7 @@ set(
test/gtest_utils.cpp
test/lib/format_int.cpp
test/lib/taint.cpp
test/small-step/binary.cpp
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do these all assume v1model in some way? Might be a flaw in the test design...

Copy link
Contributor Author

@vlstill vlstill Aug 14, 2023

Choose a reason for hiding this comment

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

They all fail with messages like this:

[ RUN      ] SmallStepTest.Unary02
unknown file: Failure
C++ exception with description "In file: /home/xstill/repo/p4c/backends/p4tools/modules/testgen/test/gtest_utils.cpp:21
Compiler Bug: Target bmv2/v1model not supported
" thrown in the test body.

Frankly, I'm not entirely sure what is the relation between BMV2 and v1model here :-).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm, maybe we should introduce a p4test like extension that works for any target at some point. Just for debugging.

The v1model1 arch is executed on bmv2. Theoretically it could also be v1model on DPDK or Tofino.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would replace "theoretically" with "hypothetically", or any other word that means "this seems very unlikely to happen, until and unless some very time-rich and motivated volunteers agree to extend P4-DPDK open source implementation to do this".

It is open source, so of course it can happen. I just don't want anyone reading this to mistake the prerequisites required to make it happen. No one I am aware of is signing up for such a project for P4-DPDK.

@vlstill vlstill added the p4tools Topics related to the P4Tools back end label Aug 14, 2023
@vlstill vlstill requested a review from fruffy August 14, 2023 18:27
@vlstill vlstill merged commit 38952df into p4lang:main Aug 15, 2023
16 checks passed
@vlstill vlstill deleted the testgen-without-bmv2 branch August 15, 2023 06:52
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.

[P4Testgen] Testgen gtest target cannot be built without BMV2 target
3 participants