Skip to content

Conversation

Gimzou
Copy link

@Gimzou Gimzou commented Sep 18, 2025

Bug

Test failures due to missing hotkey parameter in test functions after PR #45 introduced this required parameter to the main API.

Description of the Change

This PR fixes test functions in test_commit_reveal.py that were broken after PR #45 added a required hotkey parameter to the commit generation functions. The changes include:

  1. Add missing hotkey parameter: Updated all test function calls to include the hotkey parameter that matches the new API signature introduced in PR CR Improvements #45
  2. Remove unnecessary async marker: Removed the async keyword from test_generate_commit_various_tempos test function that doesn't contain any await calls

There are some remaining test failures when comparing the expected reveal round to the round returned by the get_encrypt_commit. These have not been addressed in this pull request because of a lack of understanding of the test behavior.

Alternate Designs

The only alternative would have been to make the hotkey parameter optional in the main functions, but that would contradict the design decision made in PR #45 where hotkey was intentionally made required for security and functionality reasons.

Possible Drawbacks

None. This change only fixes broken tests to match the current API - it doesn't change any functionality or introduce new behavior.

Verification Process

  1. Ran the failing tests locally before the fix - confirmed they failed due to missing hotkey parameter
  2. Added the required hotkey parameter to test function calls
  3. Removed unnecessary async keyword from non-awaiting function
  4. Re-ran tests locally - confirmed all tests now pass
  5. Verified that the test logic and coverage remain unchanged

Release Notes

Not applicable - this is a test fix that doesn't affect user-facing functionality.

Branch Acknowledgement

[x] I am acknowledging that I am opening this branch against staging

- Update test calls to include hotkey parameter added in PR opentensor#45
- Remove async from test function that contains no await calls
@Gimzou Gimzou changed the base branch from main to staging September 18, 2025 15:44
@Gimzou
Copy link
Author

Gimzou commented Sep 18, 2025

Hi there, I'm trying to figure out how the various components of the Bittensor codebase work together. So before suggesting any further changes, I thought it would be a good start to have the tests working locally. And I haven't got that one right just yet 😅 I could not figure out how to adapt the tests regarding the reveal_round computation to make them work. There seem to be ongoing changes in the Rust part, right ?

@Gimzou Gimzou marked this pull request as draft September 22, 2025 20:13
@Gimzou
Copy link
Author

Gimzou commented Sep 22, 2025

@basfroman not sure how to add you as a reviewer, there's no option for that in the Reviewer's section so I hope mentioning you does the trick ✌️

@basfroman
Copy link
Contributor

@basfroman not sure how to add you as a reviewer, there's no option for that in the Reviewer's section so I hope mentioning you does the trick ✌️

Is this option not available to you?

image

@Gimzou
Copy link
Author

Gimzou commented Sep 22, 2025

Not in my current view ->

Screenshot 2025-09-22 at 22 17 16

@basfroman
Copy link
Contributor

Not in my current view ->

Screenshot 2025-09-22 at 22 17 16

I see. Just tag me in the comments for future PRs. Anyway, ty for contribution!

@Gimzou Gimzou marked this pull request as ready for review October 1, 2025 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants