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

[docs] Added test guide #5215

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Vineet1101
Copy link
Contributor

This PR adds a comprehensive guide on testing in the P4C repository. The document covers different types of tests, their expected behavior, source file locations, how to run tests, and how to add new tests. Additionally, it includes details on continuous integration and automated test execution.

Vineet1101 and others added 2 commits April 2, 2025 16:38
@github-actions github-actions bot added the documentation Topics related to compiler documentation. label Apr 2, 2025
Copy link
Contributor

github-actions bot commented Apr 2, 2025

githubloading A preview of this PR is available at: Preview Link
📂 View the source code here: View Source Code
🔧 Commit used for deployment: bddd4423b63ecf1a474ee28626de71311ac8aaf0

Note: Changes may take a few seconds to appear on GitHub Pages. Please refresh the page if you do not see the updates immediately.

@Vineet1101
Copy link
Contributor Author

@jafingerhut there is no bmv_errors_output directory in the testdata

@jafingerhut
Copy link
Contributor

@jafingerhut there is no bmv_errors_output directory in the testdata

That is true. There is also no directory bmv_errors, either, so why would you expect there to be a directory bmv_errors_output?

Signed-off-by: Vineet1101 <Vineetgoel692@gmail.com>
@jafingerhut
Copy link
Contributor

@jafingerhut there is no bmv_errors_output directory in the testdata

That is true. There is also no directory bmv_errors, either, so why would you expect there to be a directory bmv_errors_output?

If you mean: "Why is there a directory named p4_16_bmv_errors in the testdata directory, but there is no corresponding expected outputs directory named p4_16_bmv_errors_output?" then my answer is "I don't know".

It would be good to find out, and either correct the problem, or document why one is not necessary. It might be a mistake that there is no directory p4_16_bmv_errors_output.

@Vineet1101
Copy link
Contributor Author

Vineet1101 commented Apr 2, 2025

yes I mean that why there is bmv_errors file but no output No problem I will try to find out why there is no output file

@jafingerhut there is no bmv_errors_output directory in the testdata

That is true. There is also no directory bmv_errors, either, so why would you expect there to be a directory bmv_errors_output?

If you mean: "Why is there a directory named p4_16_bmv_errors in the testdata directory, but there is no corresponding expected outputs directory named p4_16_bmv_errors_output?" then my answer is "I don't know".

It would be good to find out, and either correct the problem, or document why one is not necessary. It might be a mistake that there is no directory p4_16_bmv_errors_output.

@Vineet1101 Vineet1101 marked this pull request as ready for review April 4, 2025 02:24
@Vineet1101
Copy link
Contributor Author

Vineet1101 commented Apr 4, 2025

@jafingerhut can you please review this
also i could not find the reason why there is no output file for p4_16_bmv_errors so I have not included it in the doc for now


## Running Tests

- To run all tests: `make check -j3`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not everyone uses make. I would recommend ctest here. If make is recommended somewhere else, I would update it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added testing using ctest section and remove cmake one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also running test using make is written in the p4 docs so if you want i can update the docs

@fruffy fruffy requested a review from Copilot April 5, 2025 10:17
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

docs/AddingTestGuide.md:17

  • The source file location for Negative Tests has been removed without a replacement. Please verify if updated directory paths should be provided to ensure clarity for readers.
- `testdata/p4_16_errors/`

docs/AddingTestGuide.md:28

  • The Backend-Specific Tests section no longer lists expected source file locations. If these paths have changed, consider updating this section with the current directories to avoid confusion.
- `backends/ebpf/tests/`

Comment on lines 8 to 9
Positive tests are standard tests where the compiler should not fail due to compile-time errors. The expected output should match the actual output for the test to pass.The compiler should generate a output file with no errors.

Copy link
Preview

Copilot AI Apr 5, 2025

Choose a reason for hiding this comment

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

There is a missing space after the period in 'pass.The'; please add a space to improve readability.

Suggested change
Positive tests are standard tests where the compiler should not fail due to compile-time errors. The expected output should match the actual output for the test to pass.The compiler should generate a output file with no errors.
Positive tests are standard tests where the compiler should not fail due to compile-time errors. The expected output should match the actual output for the test to pass. The compiler should generate a output file with no errors.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

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

@Vineet1101 Vineet1101 requested review from fruffy and Copilot April 5, 2025 18:55
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

docs/AddingTestGuide.md:40

  • [nitpick] The sentence seems a bit awkward; consider rephrasing it for clarity, for example: 'Similarly, for other error output files, please refer to the corresponding directories.'
Similarly for other error output files you can check 

docs/AddingTestGuide.md:25

  • [nitpick] Consider capitalizing 'ubpf' to 'UBPF' for consistency with 'eBPF' to improve clarity.
Some test programs exist in backend-specific directories, such as eBPF, ubpf tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Topics related to compiler documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants