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

Move unit tests to the right place #1011

Merged
merged 4 commits into from
Nov 21, 2024
Merged

Conversation

ales-otf
Copy link
Contributor

@ales-otf ales-otf commented Nov 21, 2024

Description

This PR moves the tests from pallet-subtensor and pallet-admin-utils to the place, where Rust will see them as unit tests. That means, to make conditional compilation (cfg(test)) as well as access to private API and data fields in tests available. Currently these tests are located so, that Rust sees them as integration tests, which forces us to add public APIs needed to test private API.

Basically, the changes are very simple:

  • pallets/subtensor/tests moved to pallets/subtensor/src/tests
  • pallets/admin-utils/tests moved to pallets/admin-utils/src/tests

This PR will allow further codebase clean ups. Particularly, removing the public API, which exists only for testing.

Related Issue(s)

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Other (please describe):

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have run cargo fmt and cargo clippy to ensure my code is formatted and linted correctly
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@ales-otf ales-otf force-pushed the chore/move-unit-tests branch from 6284afc to 5540907 Compare November 21, 2024 17:22
@sam0x17 sam0x17 marked this pull request as ready for review November 21, 2024 17:49
@sam0x17 sam0x17 requested a review from unconst as a code owner November 21, 2024 17:49
Copy link
Contributor

@gztensor gztensor left a comment

Choose a reason for hiding this comment

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

I like this change: Now tests have access to private functions and cfg(test) also works.

@unconst unconst merged commit a84af10 into devnet-ready Nov 21, 2024
@ales-otf ales-otf deleted the chore/move-unit-tests branch November 21, 2024 19:29
@ales-otf ales-otf self-assigned this Nov 27, 2024
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.

4 participants