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

Add Unit Tests for shared_utils.py #309

Merged
merged 7 commits into from
Nov 4, 2024
Merged

Conversation

TraXIcoN
Copy link
Contributor

This PR adds unit tests for the shared_utils.py module. It covers encoding utilities, bit manipulation, argument handling, ISA type validation, string manipulation, and instruction processing. These tests will help ensure the module works correctly and is easier to maintain.

@IIITM-Jay
Copy link
Member

@TraXIcoN I have fixed the issue and removed the label do-not-merge. Will review it soon.

Looping @aswaterman and @rpsene also.

Thanks @TraXIcoN for this PR and adding test cases for shared module and its associated methods!

@IIITM-Jay
Copy link
Member

IIITM-Jay commented Nov 2, 2024

@TraXIcoN LGTM.

Definitely it contains well organized test suites comprising both positive and negative test cases. Once you resolves with conflicts, comment here so that it could get merged.

Let me know, if you run into an issue while fixing pyright things, will fix those.

test.py Outdated Show resolved Hide resolved
Copy link
Member

@IIITM-Jay IIITM-Jay left a comment

Choose a reason for hiding this comment

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

@TraXIcoN I have resolved the conflicts and fixed the pyright issues. Actually, the pre-commit tool for this repo is now also includes the pyright hooks. So, some changes were needed for test suites too, I have done the necessary changes accordingly.

@aswaterman requesting your feedback and an overview for this PR, and then I think this PR is good to be merged.

Thanks once again @TraXIcoN for the tests and @aswaterman for your extreme support!

Signed-off-by: Jay Dev Jha <[email protected]>
Copy link
Member

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

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

Yes, I'm happy with this. Thanks, @TraXIcoN.

@aswaterman aswaterman merged commit 8c459ce into riscv:master Nov 4, 2024
2 checks passed
Myrausman pushed a commit to Myrausman/riscv-opcodes that referenced this pull request Nov 14, 2024
* Added test cases for shared_utils

Signed-off-by: Aditya Mohan <[email protected]>

* Added definition for logging an error shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Pre-commit fixes for shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* pyright fixes for test.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Minor changes to shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Updated test.py

Signed-off-by: Jay Dev Jha <[email protected]>

---------

Signed-off-by: Aditya Mohan <[email protected]>
Signed-off-by: Jay Dev Jha <[email protected]>
Co-authored-by: Jay Dev Jha <[email protected]>
Myrausman pushed a commit to Myrausman/riscv-opcodes that referenced this pull request Nov 14, 2024
* Added test cases for shared_utils

Signed-off-by: Aditya Mohan <[email protected]>

* Added definition for logging an error shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Pre-commit fixes for shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* pyright fixes for test.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Minor changes to shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Updated test.py

Signed-off-by: Jay Dev Jha <[email protected]>

---------

Signed-off-by: Aditya Mohan <[email protected]>
Signed-off-by: Jay Dev Jha <[email protected]>
Co-authored-by: Jay Dev Jha <[email protected]>
Myrausman pushed a commit to Myrausman/riscv-opcodes that referenced this pull request Nov 14, 2024
* Added test cases for shared_utils

Signed-off-by: Aditya Mohan <[email protected]>

* Added definition for logging an error shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Pre-commit fixes for shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* pyright fixes for test.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Minor changes to shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Updated test.py

Signed-off-by: Jay Dev Jha <[email protected]>

---------

Signed-off-by: Aditya Mohan <[email protected]>
Signed-off-by: Jay Dev Jha <[email protected]>
Co-authored-by: Jay Dev Jha <[email protected]>
Myrausman pushed a commit to Myrausman/riscv-opcodes that referenced this pull request Nov 14, 2024
* Added test cases for shared_utils

Signed-off-by: Aditya Mohan <[email protected]>

* Added definition for logging an error shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Pre-commit fixes for shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* pyright fixes for test.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Minor changes to shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Updated test.py

Signed-off-by: Jay Dev Jha <[email protected]>

---------

Signed-off-by: Aditya Mohan <[email protected]>
Signed-off-by: Jay Dev Jha <[email protected]>
Co-authored-by: Jay Dev Jha <[email protected]>
Myrausman pushed a commit to Myrausman/riscv-opcodes that referenced this pull request Nov 14, 2024
* Added test cases for shared_utils

Signed-off-by: Aditya Mohan <[email protected]>

* Added definition for logging an error shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Pre-commit fixes for shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* pyright fixes for test.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Minor changes to shared_utils.py

Signed-off-by: Jay Dev Jha <[email protected]>

* Updated test.py

Signed-off-by: Jay Dev Jha <[email protected]>

---------

Signed-off-by: Aditya Mohan <[email protected]>
Signed-off-by: Jay Dev Jha <[email protected]>
Co-authored-by: Jay Dev Jha <[email protected]>
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.

3 participants