add regression test for "zpool list -p"

[pcd1193182]

Signed-off-by: Paul Dagnelie [email protected]

Motivation and Context

Other than this test, zpool list -p is not well tested by any of the automated tests.

Description

Add a test for zpool list -p

How Has This Been Tested?

Ran the test on Linux

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:

• x ] 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.
• All new and existing tests passed.
• All commit messages are properly formatted and contain Signed-off-by.

[sdimitro]

Code LGTM in general. I have 2 questions:

I am a bit curious about the failure here though:http://build.zfsonlinux.org/builders/CentOS%206%20x86_64%20%28TEST%29/builds/7655/steps/shell_9/logs/summary

...
Test: /usr/share/zfs/zfs-tests/tests/functional/cli_root/zpool_get/zpool_get_005_pos (run as root) [00:00] [FAIL]
21:28:58.32 sudo: unable to execute /usr/share/zfs/zfs-tests/tests/functional/cli_root/zpool_get/zpool_get_005_pos.ksh: No such file or directory
...

Is there a permission issue with the file?

One more question: Do we really need a whole new file (zpool_get_parsable.cfg) for just a list of strings (properties) that is used for only one test? Would it make more sense to just add those in zpool_get_005_pos.ksh?

[pcd1193182]

I'm not sure why the test failed on CentOS like that, and nowhere else... The permissions all look right, and the file is listed in the makefile, same as all the others. If it was an issue that basic I wouldn't think it would be platform specific?

As for the zpool_get_parsable, I didn't write the original patch, so if @prakashsurya wants to offer a different opinion he should feel free, but the way I understand it the list in that file isn't actually specific to this test; any test that wants to work with -p will want it, because it's the list of parsable properties. Since it's a semi-global list, it's stored outside of the tests in its own file.

[prakashsurya]

Looks like I committed the original change in 2014; I have no recollection of what I might have been thinking back then, but Paul's description seems reasonable to me.

With that said, it looks like nothing has used that file since it was added (at least nothing is currently using it), so perhaps in-lining the array, as Serapheim suggests, would be better.

Ultimately, I'm fine either way.

[codecov]

Codecov Report

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

@@            Coverage Diff             @@
##           master    #9134      +/-   ##
==========================================
- Coverage   79.33%   79.08%   -0.25%     
==========================================
  Files         400      400              
  Lines      121692   121692              
==========================================
- Hits        96542    96245     -297     
- Misses      25150    25447     +297

Flag	Coverage Δ
#kernel	79.67% <ø> (-0.14%)	⬇️
#user	66.83% <ø> (-0.48%)	⬇️

---

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 c81f179...c8750e0. Read the comment docs.

[pcd1193182]

Update: The CentOS failure was probably caused by the #! line being different in this test than in the others; on other platforms, ksh could be found in either location, but on CentOS it's only in /bin/.