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

zts: Fix and change testcase cache_010_neg #10172

Merged
merged 1 commit into from
Apr 13, 2020

Conversation

spaghetti-
Copy link
Contributor

@spaghetti- spaghetti- commented Apr 2, 2020

Motivation and Context

Commit 379ca9c removed the requirement on aux devices to be block
devices only but the test case cache_010_neg was not updated, making it
fail consistently.

WIP to get comments from maintainers and other contributors.

Description

This change changes the test to check that cache devices can be files
and character devices too. The testcase is renamed to cache_010_pos and
the exceptions for known failure removed from the test runner.

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

@behlendorf behlendorf added Status: Code Review Needed Ready for review and testing Component: Test Suite Indicates an issue with the test framework or a test case labels Apr 2, 2020
@behlendorf behlendorf requested a review from a user April 2, 2020 16:41
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks for digging in to getting this test updated.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

The test needs to be renamed in common.run, too.

I'll give you a chance to review the slog test and revise this one before I comment on the FreeBSD issues.

tests/zfs-tests/tests/functional/cache/cache_010_pos.ksh Outdated Show resolved Hide resolved
tests/zfs-tests/tests/functional/cache/cache_010_pos.ksh Outdated Show resolved Hide resolved
@behlendorf behlendorf added Status: Work in Progress Not yet ready for general review and removed Status: Code Review Needed Ready for review and testing labels Apr 7, 2020
@codecov-io
Copy link

codecov-io commented Apr 10, 2020

Codecov Report

Merging #10172 into master will decrease coverage by 0.22%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10172      +/-   ##
==========================================
- Coverage   79.40%   79.17%   -0.23%     
==========================================
  Files         385      385              
  Lines      122603   122603              
==========================================
- Hits        97350    97073     -277     
- Misses      25253    25530     +277     
Flag Coverage Δ
#kernel 79.68% <ø> (+0.05%) ⬆️
#user 65.51% <ø> (-1.04%) ⬇️
Impacted Files Coverage Δ
module/zfs/vdev_indirect.c 75.00% <0.00%> (-9.17%) ⬇️
cmd/zvol_id/zvol_id_main.c 76.31% <0.00%> (-5.27%) ⬇️
cmd/ztest/ztest.c 75.62% <0.00%> (-5.16%) ⬇️
module/lua/lmem.c 83.33% <0.00%> (-4.17%) ⬇️
module/zfs/zil.c 91.17% <0.00%> (-2.19%) ⬇️
module/zfs/arc.c 81.98% <0.00%> (-2.15%) ⬇️
lib/libzfs/libzfs_changelist.c 84.37% <0.00%> (-1.96%) ⬇️
module/zfs/vdev_raidz.c 91.39% <0.00%> (-1.86%) ⬇️
module/zfs/vdev_trim.c 94.75% <0.00%> (-1.63%) ⬇️
module/zfs/zvol.c 84.06% <0.00%> (-1.35%) ⬇️
... and 48 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36a6e23...c7fbde0. Read the comment docs.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Aside from that one issue this looks good to me. Please also rebase this on the latest master code when you refresh it.

tests/zfs-tests/tests/functional/cache/cache_010_pos.ksh Outdated Show resolved Hide resolved
@spaghetti- spaghetti- force-pushed the fix-cache-010-test branch 2 times, most recently from ab35b26 to ebfe93d Compare April 11, 2020 07:03
Commit 379ca9c removed the requirement on aux devices to be block
devices only but the test case cache_010_neg was not updated, making it
fail consistently.

This change changes the test to check that cache devices _can_ be
anything that presents a block interface. The testcase is renamed to
cache_010_pos and the exceptions for known failure removed from the test
runner.

Reported-by: Richard Elling <[email protected]>
Signed-off-by: Alex John <[email protected]>
@spaghetti- spaghetti- changed the title [WIP] zts: Fix and change testcase cache_010_neg zts: Fix and change testcase cache_010_neg Apr 11, 2020
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Work in Progress Not yet ready for general review labels Apr 13, 2020
@behlendorf behlendorf merged commit c602b35 into openzfs:master Apr 13, 2020
as-com pushed a commit to as-com/zfs that referenced this pull request Jun 20, 2020
Commit 379ca9c removed the requirement on aux devices to be block
devices only but the test case cache_010_neg was not updated, making it
fail consistently.

This change changes the test to check that cache devices _can_ be
anything that presents a block interface. The testcase is renamed to
cache_010_pos and the exceptions for known failure removed from the test
runner.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reported-by: Richard Elling <[email protected]>
Signed-off-by: Alex John <[email protected]>
Closes openzfs#10172 
(cherry picked from commit c602b35)
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Commit 379ca9c removed the requirement on aux devices to be block
devices only but the test case cache_010_neg was not updated, making it
fail consistently.

This change changes the test to check that cache devices _can_ be
anything that presents a block interface. The testcase is renamed to
cache_010_pos and the exceptions for known failure removed from the test
runner.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reported-by: Richard Elling <[email protected]>
Signed-off-by: Alex John <[email protected]>
Closes openzfs#10172
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Test Suite Indicates an issue with the test framework or a test case Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants