7519 Add smbclient tests

Review Request #916 — Created Feb. 24, 2018 and submitted

gwr
illumos-gate
7519
general

7519 Add smbclient tests

Import the SMB client test from the old STC collection.
Port those tests to STF.
Add Jilin's mmap tests.
Lots of cleanup.

Ran the new tests. Below is some sample output from the new tests.
Note that the 'SKIP' status is for tests that would do thing that
may take a long time (i.e. copying very large files) and the "-f"
option to smbclienttest tells it to skip those tests.

admin@oi-test:/tmp$ /opt/smbclient-tests/bin/smbclienttest -f -s orion
Test: /opt/smbclient-tests/tests/nsmbrc/tp_nsmbrc_001 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/nsmbrc/tp_nsmbrc_002 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/nsmbrc/tp_nsmbrc_003 (run as admin) [00:01] [FAIL]
Test: /opt/smbclient-tests/tests/nsmbrc/tp_nsmbrc_004 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/nsmbrc/tp_nsmbrc_005 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/nsmbrc/tp_nsmbrc_006 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/sharectl/tp_sharectl_001 (run as root) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/sharectl/tp_sharectl_002 (run as root) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/sharectl/tp_sharectl_003 (run as root) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/sharectl/tp_sharectl_004 (run as root) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/sharectl/tp_sharectl_005 (run as root) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/sharectl/tp_sharectl_006 (run as root) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/acl/tp_acl_001 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/acl/tp_acl_002 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/acl/tp_acl_003 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/acl/tp_acl_004 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/acl/tp_acl_005 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/cptest/tp_cptest_001 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/cptest/tp_cptest_002 (run as admin) [00:04] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/cptest/tp_cptest_003 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/cptest/tp_cptest_004 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/cptest/tp_cptest_005 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/cptest/tp_cptest_006 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/cptest/tp_cptest_007 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/cptest/tp_cptest_008 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/cptest/tp_cptest_009 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_001 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_002 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_003 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_004 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_005 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_006 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_007 (run as admin) [00:03] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_008 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_009 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_010 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_011 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/create/tp_create_012 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/error/tp_error_001 (run as admin) [00:02] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/error/tp_error_002 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/misc/tp_misc_001 (run as admin) [00:04] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/misc/tp_misc_002 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/misc/tp_misc_003 (run as admin) [00:04] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mkdir/tp_mkdir_001 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mkdir/tp_mkdir_002 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mkdir/tp_mkdir_003 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mkdir/tp_mkdir_004 (run as admin) [00:30] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mkdir/tp_mkdir_005 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mkdir/tp_mkdir_006 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mmap/tp_mmap_001 (run as admin) [00:02] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mmap/tp_mmap_002 (run as admin) [00:03] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mmap/tp_mmap_003 (run as admin) [00:02] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mmap/tp_mmap_004 (run as admin) [00:03] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mmap/tp_mmap_005 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mmap/tp_mmap_006 (run as admin) [00:03] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mmap/tp_mmap_007 (run as admin) [00:04] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mmap/tp_mmap_008 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mmap/tp_mmap_009 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mmap/tp_mmap_010 (run as admin) [00:02] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mvtest/tp_mvtest_001 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mvtest/tp_mvtest_002 (run as admin) [00:04] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mvtest/tp_mvtest_003 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mvtest/tp_mvtest_004 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/mvtest/tp_mvtest_005 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mvtest/tp_mvtest_006 (run as admin) [00:03] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/mvtest/tp_mvtest_007 (run as admin) [00:08] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/xattr/tp_xattr_001 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/xattr/tp_xattr_002 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/xattr/tp_xattr_003 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbfs/xattr/tp_xattr_004 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/xattr/tp_xattr_005 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/xattr/tp_xattr_006 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/xattr/tp_xattr_007 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/xattr/tp_xattr_008 (run as admin) [00:04] [PASS]
Test: /opt/smbclient-tests/tests/smbfs/xattr/tp_xattr_009 (run as admin) [00:20] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_001 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_002 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_003 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_004 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_005 (run as admin) [00:02] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_006 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_007 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_008 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_009 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_010 (run as admin) [00:02] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_011 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_012 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_013 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_014 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_015 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbmount/tp_smbmount_016 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_001 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_002 (run as admin) [00:02] [SKIP]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_003 (run as admin) [00:02] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_004 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_005 (run as admin) [00:03] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_006 (run as admin) [00:05] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_007 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_008 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_009 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_010 (run as admin) [00:00] [SKIP]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_011 (run as admin) [00:01] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_012 (run as admin) [00:00] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_013 (run as admin) [00:05] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_014 (run as admin) [00:04] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_015 (run as admin) [00:50] [PASS]
Test: /opt/smbclient-tests/tests/smbutil/tp_smbutil_016 (run as admin) [00:04] [PASS]

(there was one transient failure because this was a somewhat slow VM)

  • 0
  • 0
  • 66
  • 5
  • 71
Description From Last Updated
rm
  1. Thanks for putting together a thorough test suite. It's useful to have. I know there are a lot of nits here around the quality of the test code. A lot of that is to make it so that way if something goes wrong we can better understand it as a lot of this isn't going to be easy to debug. A lot of the notes here apply to large numbers of files.

    1. Yeah... all of Jilin's tests could use some cleanup.
      I really didn't want to take the time on that right now.
      I'm really busy on the SMB2 stuff for the next month.
      If I really have to clean up this test code, that will
      probably mean this stuff will have to wait a while.

  2. Probably want $(POST_PROCESS_O) here.

  3. close can clobber the errno here.
  4. Looks like this file isn't cstyle clean.
  5. This file isn't cstyle clean.

    1. Yeah... all of Jilin's tests could use some cleanup.
      I really didn't want to take the time on that right now.
      I'm really busy on the SMB2 stuff for the next month.
      If I really have to clean up this test code, that will
      probably mean this stuff will have to wait a while.

  6. Should probably be unsigned.

  7. Should probably be unsigned values.

  8. Is there a reason we're not using getopt?

    1. non-standard usage (option with two opt args)

  9. Make explicit comparison or change to boolean_t?
  10. Is there something outside of the test that makes sure that the file we're passing in isn't a zero-byte file? Should we check that here?
  11. This program isn't cstyle clean
  12. Probably should be unsigned

  13. Is there a reason ew're not using getopt?

  14. May be worth using a form that we can actually check errors from. Maybe strtonum() now is the best since we don't care about the base.

  15. This is a bit ugly (imo)
  16. Should probably be unsigned

  17. Why can't we use getopts?

    1. multiple optarg parts for each option
      (and I don't feel like redesigning that)

  18. usr/src/test/smbclient-tests/cmd/prot_mmap/prot_mmap.c (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
    What happens if both files are zero size? Should the test guard against that?
  19. Should be unsigned, we never want to go negative / overflow. I'd probably just make it a uint64_t

  20. Can we please use getopts?

  21. Need to check for malloc failure.

  22. Don't we want to check the read return value before using it here? If this was -1, this would blow up in a hurry.

  23. Same return value checking note

  24. Check return value

  25. usr/src/test/smbclient-tests/doc/README (Diff revision 1)
     
     

    A missing prerequiesite is making sure that expect or something else is installed.

  26. Clean up white space noise in the file?

  27. If we're using the -s option, why do we need to check the state with the timeout logic?

    1. Not sure. nuked that stuff.

  28. If we're using the -s option, why do we need to check the state with the timeout logic?

  29. Missing space between func and arg?

  30. Can this all be replaced with the explicit mount options?

  31. I think you just want pkill -u <pid> -x smbiod
    1. got rid of this, now killing connections instead.
      (Killing IOD can cause the svc to go into maintenance)

  32. Can we use modern tests, eg. '[['

  33. Do you care if this ls/awk, etc fail?

  34. Erm, 80 char lines?
  35. trim trailing whitespace?

  36. What if it's not a file, but it exists?

  37. trim trailing whitespace?
  38. What if these fail?

  39. What if these fail?

  40. Is there a script that helps clean up these things if we ultimately fail while in test context so we can run again?

    Perhaps we can add a note about potential system side effects to the README?

    1. Oh, actually, configure.ksh was in the old test, but I didn't use it.
      I'm not sure whether to keep it, make it a README, kill it, or what...

    2. Just removed it.

    1. yeah, configure.ksh was all a "todo" I forgot about.

  41. usr/src/test/smbclient-tests/tests/nsmbrc/tp_nsmbrc_003.ksh (Diff revision 1)
     
     
     
     
     
     
     
     

    Check return values / empty results?

  42. usr/src/test/smbclient-tests/tests/nsmbrc/tp_nsmbrc_004.ksh (Diff revision 1)
     
     
     
     
     
     
     
     
     

    Check return values / empty results?

  43. usr/src/test/smbclient-tests/tests/nsmbrc/tp_nsmbrc_005.ksh (Diff revision 1)
     
     
     
     
     
     
     

    Check return values / empty results?

  44. usr/src/test/smbclient-tests/tests/nsmbrc/tp_nsmbrc_006.ksh (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     

    Check return values / empty results?

  45. There's no copyright holder here? Can we maybe use the new cddl header for the tp_mmap_*.ksh tests?
  46. Trim trailing whitespace in this file?

  47. Can we at least make sure we have 80 columns in comments please?

  48. This probably needs to be dealt with as a THIRDPARTYLICENE and a THIRDPARTYLICENSE.descrip that is also mentioned in the package manifest.

    1. I added a LICENSE file in rev 2 of this RB.
      Unfortunately, it's the "Artistic license", which restricts what kinds of changes one can make
      before having an obligation to provide both our modified version and the "Starndard Version".
      I'd rather avoid running afoul of those terms and keep the changes to "portability".

  49. Any background on this XXX?

    1. I'm not sure. That was in the version Sun distributed.

  50. If we're always going to do it, let's just remove the conditional.

    1. and change all the indentation, and ...?
      (see my notes above about minimizing changes)

    2. Just put that back as it was.

  51. usr/src/test/test-runner/stf/contrib/include/ctiutils.shlib (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    a heredoc may be easier to use here

    1. Yeah, though the "Artistic" license really discourages modifications.
      I'm going to do a little more work in here to make clear that we're
      in compliance with the annoying license terms.

  52. 
      
gwr
gwr
gwr
tsoome
  1. 
      
  2. red ugliness.

  3. 
      
gwr
gwr
gwr
rm
  1. Thanks for all the clean up here. I'm generally happy with how things changed here. I appreciate all the work that went into this.

    1. Just to be clear, most of the questions I have here aren't blockers or things that should change. Just trying to understand some of the sudo changes which occurred in a lot more places than those mentioned.
    2. These tests used to all run as root, and many did "su $user -c ..."
      I changed most of them to run as a normal user, but a few things
      then required sudo.

  2. Is there a reason this now requires sudo?
    1. umount with -f required priv.

  3. Another case where it seems sudo is now required?
    1. chown required priv.

  4. Could be $PWD, but doesn't matter.
    1. I thought I got rid of all the "cd somewhere; do things; cd back" logic,
      so perhaps I should look at this one again.

    2. Yeah, that CDIR variable is unused now. missed that in cleanup.

  5. 
      
gwr
gwr
gwr
Review request changed

Status: Closed (submitted)

Loading...