mbox series

[blktests,0/4] Cleanup and Optimization in requires()

Message ID 20250107061905.91316-1-lizhijian@fujitsu.com (mailing list archive)
Headers show
Series Cleanup and Optimization in requires() | expand

Message

Zhijian Li (Fujitsu) Jan. 7, 2025, 6:19 a.m. UTC
(Most of the this cover letter was generated by the chatgpt)
Hello,

This patch series aims to enhance the efficiency and clarity of the
requires() by optimizing the way module driver and remove unnecessary '&&'

Here's a brief summary of each patch included in this series:

1. **Patch 1/4: common/rc: Test `have_driver` before checking its driver parameter**
   - This patch optimizes the `_have_module_param()` function by first
verifying the presence of a driver with `_have_driver()`. This change reduces
unnecessary checks and error logs when a driver is not available, improving
test efficiency.

2. **Patch 2/4: tests, common: Get rid of `_have_null_blk`**
   - The `_have_null_blk` function has been removed since it duplicates the
functionality of `_have_driver null_blk`. This helps streamline the
codebase by eliminating redundancy.

3. **Patch 3/4: common, new, tests: Get rid of `_have_scsi_debug`**
   - Similar to the previous patch, `_have_scsi_debug` was also redundant.
This patch removes it to ensure consistency and reduce code complexity.

4. **Patch 4/4: tests: Remove unnecessary '&&' in `requires()` functions**
   - This patch refactors `requires()` functions to evaluate conditions
independently. The change aims to improve diagnostic clarity by displaying
all unmet conditions and their respective skip reasons.

These changes should collectively improve the test scripts' readability
and efficiency, making module checks faster and less error-prone.
Please review the patches and provide feedback.

Thank you for your time and consideration.

Best regards,
Li Zhijian


Li Zhijian (4):
  common/rc: test have_driver before check its driver parameter
  tests, common: Get rid of _have_null_blk
  common, new, tests: Get rid of _have_scsi_debug
  tests: Remove unnecessary '&&' in requires() functions

 common/null_blk   | 4 ----
 common/rc         | 6 ++----
 common/scsi_debug | 4 ----
 new               | 2 +-
 tests/block/001   | 2 +-
 tests/block/002   | 2 +-
 tests/block/006   | 3 ++-
 tests/block/008   | 3 ++-
 tests/block/010   | 3 ++-
 tests/block/011   | 3 ++-
 tests/block/014   | 2 +-
 tests/block/015   | 2 +-
 tests/block/016   | 2 +-
 tests/block/017   | 2 +-
 tests/block/018   | 2 +-
 tests/block/019   | 3 ++-
 tests/block/020   | 3 ++-
 tests/block/021   | 2 +-
 tests/block/022   | 2 +-
 tests/block/023   | 2 +-
 tests/block/024   | 2 +-
 tests/block/027   | 2 +-
 tests/block/029   | 3 ++-
 tests/block/030   | 2 +-
 tests/block/031   | 2 +-
 tests/block/037   | 2 +-
 tests/block/038   | 2 +-
 tests/loop/002    | 4 +++-
 tests/nbd/001     | 4 +++-
 tests/nbd/002     | 3 ++-
 tests/nbd/003     | 3 ++-
 tests/nvme/005    | 3 ++-
 tests/nvme/010    | 3 ++-
 tests/nvme/039    | 4 ++--
 tests/nvme/056    | 4 +++-
 tests/scsi/001    | 3 ++-
 tests/scsi/002    | 3 ++-
 tests/scsi/004    | 2 +-
 tests/scsi/005    | 1 -
 tests/throtl/rc   | 2 +-
 tests/zbd/rc      | 2 +-
 41 files changed, 59 insertions(+), 51 deletions(-)

Comments

Shin'ichiro Kawasaki Jan. 16, 2025, 2:44 a.m. UTC | #1
On Jan 07, 2025 / 14:19, Li Zhijian wrote:
> (Most of the this cover letter was generated by the chatgpt)
> Hello,
> 
> This patch series aims to enhance the efficiency and clarity of the
> requires() by optimizing the way module driver and remove unnecessary '&&'
> 
> Here's a brief summary of each patch included in this series:
> 
> 1. **Patch 1/4: common/rc: Test `have_driver` before checking its driver parameter**
>    - This patch optimizes the `_have_module_param()` function by first
> verifying the presence of a driver with `_have_driver()`. This change reduces
> unnecessary checks and error logs when a driver is not available, improving
> test efficiency.
> 
> 2. **Patch 2/4: tests, common: Get rid of `_have_null_blk`**
>    - The `_have_null_blk` function has been removed since it duplicates the
> functionality of `_have_driver null_blk`. This helps streamline the
> codebase by eliminating redundancy.
> 
> 3. **Patch 3/4: common, new, tests: Get rid of `_have_scsi_debug`**
>    - Similar to the previous patch, `_have_scsi_debug` was also redundant.
> This patch removes it to ensure consistency and reduce code complexity.
> 
> 4. **Patch 4/4: tests: Remove unnecessary '&&' in `requires()` functions**
>    - This patch refactors `requires()` functions to evaluate conditions
> independently. The change aims to improve diagnostic clarity by displaying
> all unmet conditions and their respective skip reasons.
> 
> These changes should collectively improve the test scripts' readability
> and efficiency, making module checks faster and less error-prone.
> Please review the patches and provide feedback.

Hi Li, thanks for the patches. I'm all for the 1st and 4th patches, but rather
reluctant to agree with the 2nd and 3rd patches. IMO, "_have_null_blk" and
"_have_scsi_debug" are not so bad since they need less types, and a bit more
comfortable to read than "_have_driver scsi_debug/null_blk".

I already applied the 1st patch (thanks!). If you agree with my comment above,
rework for the 4th patch will be appreciated.