Skip to content
This repository has been archived by the owner on Feb 5, 2025. It is now read-only.

fix: gate no_storage import within test scope #744

Conversation

0xObsidian
Copy link

This PR:

  • Fixes linter errors by properly gating no_storage import within test scope
  • Ensures no_storage module is only imported when the feature flag is enabled

Key places to review:

  • src/availability.rs
  • Feature gate implementation

Things tested

  • Verified linter errors are resolved
  • Confirmed all tests pass with cargo nextest run --all-features

Implementation Details

The no_storage module is feature-gated behind no-storage, but was being imported at the test module level, causing linter errors when the feature was not enabled. The fix:

  • Moved no_storage import inside the specific test that uses it
  • Added feature gate to the test: #[cfg(all(not(target_os = "windows"), feature = "no-storage"))]
  • Kept other tests in the module accessible without the feature

Previous Error:

unresolved import `crate::data_source::storage::no_storage`

@CLAassistant
Copy link

CLAassistant commented Dec 3, 2024

CLA assistant check
All committers have signed the CLA.

@0xObsidian 0xObsidian marked this pull request as draft December 3, 2024 22:50
@0xObsidian 0xObsidian force-pushed the 0xObsidian/import-within-test-scope branch from 58a5268 to 14554cb Compare December 3, 2024 22:56
@0xObsidian 0xObsidian marked this pull request as ready for review December 3, 2024 22:57
@imabdulbasit
Copy link
Contributor

imabdulbasit commented Dec 4, 2024

Thank you for your contribution. Please merge main or rebase to fix clippy errors on CI

@0xObsidian
Copy link
Author

Thank you for your contribution. Please merge main or rebase to fix clippy errors on CI

Hi @imabdulbasit Thanks for the reply, I will do it today evening

The `no_storage` module is feature-gated behind `no-storage`,
but was being imported at the test module level, causing
linter errors when the feature was not enabled.
- Linter was failing with error:
  unresolved import `crate::data_source::storage::no_storage`

Implemented Fix
---------------
- Moved no_storage import inside the specific test that uses it
- Added feature gate to the test:
  `#[cfg(all(not(target_os = "windows"), feature = "no-storage"))]`
- Kept other tests in the module accessible without the feature

Testing
-------
- Verified linter errors are resolved
- Confirmed all tests pass with `cargo nextest run --all-features`
@0xObsidian 0xObsidian force-pushed the 0xObsidian/import-within-test-scope branch from 14554cb to 3cfca56 Compare December 4, 2024 21:05
@0xObsidian
Copy link
Author

Hi @imabdulbasit, it's done

@imabdulbasit
Copy link
Contributor

We have removed the NoStorage feature completely so I am going to close this PR

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

Successfully merging this pull request may close these issues.

3 participants