Skip to content

[Security] Harden test assertions (#933)#938

Open
ShadowRoot07 wants to merge 13 commits intocashubtc:mainfrom
ShadowRoot07:security/harden-test-assertions
Open

[Security] Harden test assertions (#933)#938
ShadowRoot07 wants to merge 13 commits intocashubtc:mainfrom
ShadowRoot07:security/harden-test-assertions

Conversation

@ShadowRoot07
Copy link
Copy Markdown

Closes #933. This PR replaces simple assertions in test files with descriptive pytest assertions. This ensures that even when Python is run with optimizations (-O), the tests will not be bypassed. Manual verification of all 9 instances performed.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

was this committed by mistake. it's not setting up the poetry env and it has spanish names/comments

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, that was a mistake on my part. I was testing the environment logic in my local mobile setup and accidentally included that workflow in the commit. I'll remove the redundant workflow and the extra documentation to keep this PR focused strictly on the test assertions for issue #933. Thanks for catching that!

SH-READNE.md Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This also shouldn't be here

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The initial idea was to run a matrix of Python versions (3.10 to 3.13) to catch any version-specific issues, but since the project already has a robust CI, I'll remove these extra files to keep the PR focused on the security assertions.

SH-READNE.md Outdated
@@ -0,0 +1,12 @@
## Overview
This PR addresses issue #933 by hardening assertions in the core test suite and introducing a dedicated GitHub Actions workflow (`shadow_tests`) to ensure stability across Python versions.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

to ensure stability across Python versions

What does the workflow do to ensure that?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're right, my apologies. I included that workflow while testing the environment locally in my mobile setup and forgot to adapt it to the project's Poetry standards and language. I will remove the redundant workflow and focus strictly on the test assertions hardening as requested in issue #933. I'll clean up the commit shortly.

@KvngMikey
Copy link
Copy Markdown
Contributor

I think we should close this as not planned just as the issue was closed, + there's really no credible code change made in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Security] Multiple Test Assertion Issues - Code Quality Improvements

3 participants