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

Clean up BMv2's run-stf-test script and integrate it with testutils #4981

Merged
merged 2 commits into from
Oct 28, 2024

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Oct 24, 2024

This PR refactors the run-bmv2-test.py script in the BMv2 folder. The old script was hard to maintain and didn't use a lot of the tooling we now have available.

The new version has types and uses argparse properly. It also calls into the common logging facilities provided by testutils. The script is more compact and simpler to edit now.

@jafingerhut After this refactor, we can also add #4906 on top of it. I will take care of it.

@fruffy fruffy added the bmv2 Topics related to BMv2 or v1model label Oct 24, 2024
@fruffy fruffy force-pushed the fruffy/run-stf-test-refactor branch 2 times, most recently from c91fd73 to 816b35a Compare October 24, 2024 21:21
@fruffy fruffy added the infrastructure Topics related to code style and build and test infrastructure. label Oct 24, 2024
@fruffy fruffy force-pushed the fruffy/run-stf-test-refactor branch 6 times, most recently from 1b54e21 to 85f3eea Compare October 25, 2024 21:47
@fruffy fruffy marked this pull request as ready for review October 25, 2024 21:47
[tool.pylint.main]
init-hook="from pylint.config import find_default_config_files; import os, sys; sys.path.append(os.path.dirname(next(find_default_config_files())))"
init-hook="from pylint.config import find_default_config_files; import os, sys, pathlib; sys.path.append(next(find_default_config_files()).parent);sys.path.append(f'{pathlib.Path().home()}.local/lib/python{sys.version_info[0]}.{sys.version_info[1]}/site-packages/');"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I noticed the linters were missing some paths. This command adds them. I can make this a separate PR, if needed.

@fruffy fruffy requested a review from jafingerhut October 27, 2024 15:35
@fruffy fruffy added the run-ubuntu18 Use this tag to trigger a Ubuntu-18 CI run. label Oct 27, 2024
@fruffy fruffy force-pushed the fruffy/run-stf-test-refactor branch from 85f3eea to 4d61010 Compare October 28, 2024 13:59
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.

I at least read through the changes, and nothing seems objectionable. The tests all still pass, so that is good. I won't claim to have done the level of review that I would have caught anything extremely subtle.

@fruffy fruffy added this pull request to the merge queue Oct 28, 2024
Merged via the queue into main with commit e795b84 Oct 28, 2024
19 checks passed
@fruffy fruffy deleted the fruffy/run-stf-test-refactor branch October 28, 2024 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bmv2 Topics related to BMv2 or v1model infrastructure Topics related to code style and build and test infrastructure. run-ubuntu18 Use this tag to trigger a Ubuntu-18 CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants