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: functional/mount scripts are not removing /var/tmp/testdir.X directories #16915

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

tsoome
Copy link
Contributor

@tsoome tsoome commented Jan 2, 2025

cleanup.ksh is assuming we have TESTDIRS set [it is not].

Motivation and Context

Clean up left over files after zfstests.

Description

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)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

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

@amotin
Copy link
Member

amotin commented Jan 2, 2025

@tsoome Please fix too long line in commit message.

cleanup.ksh is assuming we have TESTDIRS set.

Signed-off-by: Toomas Soome <[email protected]>
@amotin amotin added the Status: Code Review Needed Ready for review and testing label Jan 2, 2025
@tonyhutter
Copy link
Contributor

Yea, I don't see TESTDIRS defined anywhere. I wonder if we can remove these as well?

~/zfs $ fgrep -RIn TESTDIRS 
tests/zfs-tests/tests/functional/mount/cleanup.ksh:36:for dir in $TESTDIRS; do
tests/zfs-tests/tests/functional/pam/cleanup.ksh:30:log_must rm -rf "$runstatedir" $TESTDIRS

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.

I don't see TESTDIRS defined anywhere. I wonder if we can remove these as well?

Indeed. We should remove it from tests/zfs-tests/tests/functional/pam/cleanup.ksh as well. At least in this case it's harmless since it will expand to nothing. Still, something we should cleanup.

@tsoome
Copy link
Contributor Author

tsoome commented Jan 2, 2025

I don't see TESTDIRS defined anywhere. I wonder if we can remove these as well?

Indeed. We should remove it from tests/zfs-tests/tests/functional/pam/cleanup.ksh as well. At least in this case it's harmless since it will expand to nothing. Still, something we should cleanup.

I'd leave that for separate update, so it is easier for me to keep track with illumos changes.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jan 2, 2025
@behlendorf behlendorf merged commit d6b4110 into openzfs:master Jan 2, 2025
23 of 25 checks passed
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Jan 3, 2025
cleanup.ksh is assuming we have TESTDIRS set.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Toomas Soome <[email protected]>
Closes openzfs#16915
@tsoome tsoome deleted the testdir branch January 3, 2025 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants