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

Improve compilesim.py test coverage #458

Open
wants to merge 4 commits into
base: development
Choose a base branch
from

Conversation

gaborszita
Copy link

I added more tests to improve the test coverage of compilesim.py:

  • Added tests test_less_than_cmp_simulation and test_equals_simulation to cover functions _build_eq and _build_cmp in compilesim.py
  • Added tests to test RomBlocks with more bit widths to cover the function _romwidth in compilesim.py
  • Added test test_rom_romblock_in_memory_value_map_error to test the exception raise PyrtlError('RomBlock in memory_value_map') (line 781 in compilesim.py)

Copy link

codecov bot commented Jan 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.7%. Comparing base (7dfa485) to head (9c2b493).

Additional details and impacted files
@@              Coverage Diff              @@
##           development    #458     +/-   ##
=============================================
+ Coverage         91.4%   91.7%   +0.4%     
=============================================
  Files               24      24             
  Lines             6413    6413             
=============================================
+ Hits              5858    5878     +20     
+ Misses             555     535     -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@fdxmw fdxmw left a comment

Choose a reason for hiding this comment

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

Thank you for the pull request, looks good overall! I have some minor style comments, and there are a couple pycodestyle errors in the presubmit checks too:

  tests/test_compilesim.py:948:1: W293 blank line contains whitespace
  tests/test_compilesim.py:951:1: W293 blank line contains whitespace

tests/test_compilesim.py Outdated Show resolved Hide resolved
tests/test_compilesim.py Outdated Show resolved Hide resolved
tests/test_compilesim.py Outdated Show resolved Hide resolved
tests/test_compilesim.py Outdated Show resolved Hide resolved
@gaborszita
Copy link
Author

@fdxmw fixed

@gaborszita gaborszita requested a review from fdxmw January 6, 2025 12:15
@@ -966,6 +990,18 @@ def rom_data_function(add):
("o2", lambda x: rom_data_function(2 * x))), 6)
self.compareIO(self.sim_trace, exp_out)

def test_function_rom_block_with_optimization_bidwidth_4(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry but there are still bidwidths in the names of these four tests

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

Successfully merging this pull request may close these issues.

2 participants