- Download Diff

Summary:

7519 Add smbclient tests

Review Request #916 — Created Feb. 24, 2018 and submitted Oct. 16, 2018, 1 p.m.

Information
Owner:	gwr
Repository:	illumos-gate
Branch:
Bugs:	7519
Depends On:
Reviewers
Groups:	general
People:

Description

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

Testing Done

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)

Issues

0
0
66
5
All Issues: 71

Description	From	Last Updated
Probably want $(POST_PROCESS_O) here.	rm	April 10, 2018, 1:13 p.m.
close can clobber the errno here.	rm	April 10, 2018, 1:13 p.m.
Looks like this file isn't cstyle clean.	rm	April 13, 2018, 9:13 p.m.
This file isn't cstyle clean.	rm	April 13, 2018, 9:13 p.m.
Should probably be unsigned.	rm	April 10, 2018, 1:17 p.m.
Should probably be unsigned values.	rm	April 10, 2018, 1:19 p.m.
Is there a reason we're not using getopt?	rm	April 10, 2018, 1:40 p.m.
Make explicit comparison or change to boolean_t?	rm	April 10, 2018, 1:42 p.m.
Is there something outside of the test that makes sure that the file we're passing in isn't a zero-byte file? ...	rm	April 10, 2018, 1:45 p.m.
This program isn't cstyle clean	rm	April 10, 2018, 2:21 p.m.
Probably should be unsigned	rm	April 10, 2018, 2:21 p.m.
Is there a reason ew're not using getopt?	rm	April 10, 2018, 2:21 p.m.
May be worth using a form that we can actually check errors from. Maybe strtonum() now is the best since ...	rm	April 10, 2018, 2:22 p.m.
cstyle	rm	April 10, 2018, 2:25 p.m.
cstyle	rm	April 10, 2018, 2:33 p.m.
Should probably be unsigned	rm	April 10, 2018, 2:34 p.m.
Why can't we use getopts?	rm	April 10, 2018, 2:34 p.m.
What happens if both files are zero size? Should the test guard against that?	rm	April 10, 2018, 2:37 p.m.
cstyle	rm	April 10, 2018, 2:44 p.m.
Should be unsigned, we never want to go negative / overflow. I'd probably just make it a uint64_t	rm	April 10, 2018, 2:44 p.m.
Can we please use getopts?	rm	April 10, 2018, 2:44 p.m.
Need to check for malloc failure.	rm	April 10, 2018, 2:45 p.m.
Don't we want to check the read return value before using it here? If this was -1, this would blow ...	rm	April 10, 2018, 2:47 p.m.
Same return value checking note	rm	April 10, 2018, 2:47 p.m.
Check return value	rm	April 10, 2018, 2:49 p.m.
A missing prerequiesite is making sure that expect or something else is installed.	rm	April 10, 2018, 2:52 p.m.
Clean up white space noise in the file?	rm	April 10, 2018, 2:55 p.m.
If we're using the -s option, why do we need to check the state with the timeout logic?	rm	April 10, 2018, 3 p.m.
If we're using the -s option, why do we need to check the state with the timeout logic?	rm	April 10, 2018, 3 p.m.
Missing space between func and arg?	rm	April 10, 2018, 3:03 p.m.
Can this all be replaced with the explicit mount options?	rm	April 10, 2018, 3:03 p.m.
Is there a reason this now requires sudo?	rm	Oct. 16, 2018, 12:32 a.m.
I think you just want pkill -u -x smbiod	rm	April 10, 2018, 3:04 p.m.
Can we use modern tests, eg. '[['	rm	April 10, 2018, 3:32 p.m.
Do you care if this ls/awk, etc fail?	rm	April 10, 2018, 3:32 p.m.
trim trailing whitespace?	rm	April 10, 2018, 3:44 p.m.
What if it's not a file, but it exists?	rm	April 10, 2018, 3:45 p.m.
What if these fail?	rm	April 10, 2018, 3:52 p.m.
What if these fail?	rm	April 10, 2018, 3:52 p.m.
Is there a script that helps clean up these things if we ultimately fail while in test context so we ...	rm	April 10, 2018, 3:52 p.m.
Remove the XXXs?	rm	April 10, 2018, 3:52 p.m.
Check return value?	rm	April 10, 2018, 3:56 p.m.
Check return value?	rm	April 10, 2018, 3:56 p.m.
Check retun value?	rm	April 10, 2018, 3:59 p.m.
Check return values / empty results?	rm	April 10, 2018, 3:59 p.m.
Check return values / empty results?	rm	April 10, 2018, 3:59 p.m.
Check return values / empty results?	rm	April 10, 2018, 4 p.m.
Check return values / empty results?	rm	April 10, 2018, 4 p.m.
Another case where it seems sudo is now required?	rm	Oct. 16, 2018, 12:32 a.m.
There's no copyright holder here? Can we maybe use the new cddl header for the tp_mmap_*.ksh tests?	rm	April 10, 2018, 4 p.m.
Trim trailing whitespace in this file?	rm	April 10, 2018, 4 p.m.
Can we at least make sure we have 80 columns in comments please?	rm	April 10, 2018, 4:10 p.m.
This probably needs to be dealt with as a THIRDPARTYLICENE and a THIRDPARTYLICENSE.descrip that is also mentioned in the package ...	rm	Feb. 24, 2018, 7:51 p.m.
Any background on this XXX?	rm	April 10, 2018, 4:11 p.m.
If we're always going to do it, let's just remove the conditional.	rm	April 10, 2018, 4:14 p.m.
a heredoc may be easier to use here	rm	April 10, 2018, 4:15 p.m.
red ugliness.	tsoome	April 14, 2018, 3 p.m.
red ugliness.	tsoome	April 14, 2018, 3 p.m.
red ugliness.	tsoome	April 14, 2018, 3:13 p.m.
red ugliness.	tsoome	April 14, 2018, 3:13 p.m.
red ugliness.	tsoome	April 14, 2018, 3:13 p.m.
red ugliness.	tsoome	April 14, 2018, 3:13 p.m.
red ugliness.	tsoome	April 14, 2018, 3:13 p.m.
red ugliness.	tsoome	April 14, 2018, 3:14 p.m.
red ugliness.	tsoome	April 14, 2018, 3:14 p.m.
red ugliness.	tsoome	April 14, 2018, 3:14 p.m.
red ugliness.	tsoome	April 14, 2018, 3:14 p.m.
red ugliness.	tsoome	April 14, 2018, 3:14 p.m.
red ugliness.	tsoome	April 14, 2018, 3:20 p.m.
red ugliness.	tsoome	April 14, 2018, 3:20 p.m.
red ugliness.	tsoome	April 14, 2018, 3:21 p.m.

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.

gwr Feb. 24, 2018, 7:55 p.m.

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.

usr/src/test/smbclient-tests/cmd/Makefile.cmd (Diff revision 1)
Show all issues
```
Probably want $(POST_PROCESS_O) here.
```
usr/src/test/smbclient-tests/cmd/abort_conn/abort_conn.c (Diff revision 1)
Show all issues
```
close can clobber the errno here.
```
usr/src/test/smbclient-tests/cmd/close_wr/close_wr.c (Diff revision 1)
Show all issues
```
Looks like this file isn't cstyle clean.
```

usr/src/test/smbclient-tests/cmd/cp_mmap/cp_mmap.c (Diff revision 1)

Show all issues

This file isn't cstyle clean.

gwr Feb. 24, 2018, 6:21 p.m.

usr/src/test/smbclient-tests/cmd/cp_mmap/cp_mmap.c (Diff revision 1)
Show all issues
```
Should probably be unsigned.
```
usr/src/test/smbclient-tests/cmd/cp_mmap/cp_mmap.c (Diff revision 1)
Show all issues
```
Should probably be unsigned values.
```
usr/src/test/smbclient-tests/cmd/cp_mmap/cp_mmap.c (Diff revision 1)
Show all issues
```
Is there a reason we're not using getopt?
```
1. gwr April 10, 2018, 4:15 p.m.
  non-standard usage (option with two opt args)
usr/src/test/smbclient-tests/cmd/cp_mmap/cp_mmap.c (Diff revision 1)
Show all issues
```
Make explicit comparison or change to boolean_t?
```

usr/src/test/smbclient-tests/cmd/cp_mmap/cp_mmap.c (Diff revision 1)

Show all issues

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?

usr/src/test/smbclient-tests/cmd/mkfile_mmap/mkfile_mmap.c (Diff revision 1)
Show all issues
```
This program isn't cstyle clean
```
usr/src/test/smbclient-tests/cmd/mkfile_mmap/mkfile_mmap.c (Diff revision 1)
Show all issues
```
Probably should be unsigned
```
usr/src/test/smbclient-tests/cmd/mkfile_mmap/mkfile_mmap.c (Diff revision 1)
Show all issues
```
Is there a reason ew're not using getopt?
```

usr/src/test/smbclient-tests/cmd/mkfile_mmap/mkfile_mmap.c (Diff revision 1)

Show all issues

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.

usr/src/test/smbclient-tests/cmd/mkfile_mmap/mkfile_mmap.c (Diff revision 1)
```
This is a bit ugly (imo)
```
usr/src/test/smbclient-tests/cmd/no_close/no_close.c (Diff revision 1)
Show all issues
```
cstyle
```
usr/src/test/smbclient-tests/cmd/prot_mmap/prot_mmap.c (Diff revision 1)
Show all issues
```
cstyle
```
usr/src/test/smbclient-tests/cmd/prot_mmap/prot_mmap.c (Diff revision 1)
Show all issues
```
Should probably be unsigned
```
usr/src/test/smbclient-tests/cmd/prot_mmap/prot_mmap.c (Diff revision 1)
Show all issues
```
Why can't we use getopts?
```
1. gwr April 10, 2018, 4:15 p.m.
  multiple optarg parts for each option
  (and I don't feel like redesigning that)

usr/src/test/smbclient-tests/cmd/prot_mmap/prot_mmap.c (Diff revision 1)

Show all issues

What happens if both files are zero size? Should the test guard against that?

usr/src/test/smbclient-tests/cmd/rw_mmap/rw_mmap.c (Diff revision 1)
Show all issues
```
cstyle
```

usr/src/test/smbclient-tests/cmd/rw_mmap/rw_mmap.c (Diff revision 1)

Show all issues

Should be unsigned, we never want to go negative / overflow. I'd probably just make it a uint64_t

usr/src/test/smbclient-tests/cmd/rw_mmap/rw_mmap.c (Diff revision 1)
Show all issues
```
Can we please use getopts?
```
usr/src/test/smbclient-tests/cmd/rw_mmap/rw_mmap.c (Diff revision 1)
Show all issues
```
Need to check for malloc failure.
```

usr/src/test/smbclient-tests/cmd/rw_mmap/rw_mmap.c (Diff revision 1)

Show all issues

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.

usr/src/test/smbclient-tests/cmd/rw_mmap/rw_mmap.c (Diff revision 1)
Show all issues
```
Same return value checking note
```
usr/src/test/smbclient-tests/cmd/rw_mmap/rw_mmap.c (Diff revision 1)
Show all issues
```
Check return value
```

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

Show all issues

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

usr/src/test/smbclient-tests/include/services_common.ksh (Diff revision 1)
Show all issues
```
Clean up white space noise in the file?
```

usr/src/test/smbclient-tests/include/services_common.ksh (Diff revision 1)

Show all issues

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

gwr April 10, 2018, 4:15 p.m.
```
Not sure.  nuked that stuff.
```

usr/src/test/smbclient-tests/include/services_common.ksh (Diff revision 1)

Show all issues

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

usr/src/test/smbclient-tests/include/smbmount_common.ksh (Diff revision 1)
Show all issues
```
Missing space between func and arg?
```
usr/src/test/smbclient-tests/include/smbmount_common.ksh (Diff revision 1)
Show all issues
```
Can this all be replaced with the explicit mount options?
```

usr/src/test/smbclient-tests/include/smbutil_common.ksh (Diff revision 1)

Show all issues

I think you just want pkill -u <pid> -x smbiod

gwr April 10, 2018, 4:15 p.m.

got rid of this, now killing connections instead.
(Killing IOD can cause the svc to go into maintenance)

usr/src/test/smbclient-tests/include/utils_common.ksh (Diff revision 1)
Show all issues
```
Can we use modern tests, eg. '[['
```
usr/src/test/smbclient-tests/include/utils_common.ksh (Diff revision 1)
Show all issues
```
Do you care if this ls/awk, etc fail?
```
usr/src/test/smbclient-tests/include/xattr_common.ksh (Diff revision 1)
```
Erm, 80 char lines?
```
usr/src/test/smbclient-tests/include/xattr_common.ksh (Diff revision 1)
Show all issues
```
trim trailing whitespace?
```
usr/src/test/smbclient-tests/include/xattr_common.ksh (Diff revision 1)
Show all issues
```
What if it's not a file, but it exists?
```
usr/src/test/smbclient-tests/tests/Makefile (Diff revision 1)
```
trim trailing whitespace?
```
usr/src/test/smbclient-tests/tests/configure/configure.ksh (Diff revision 1)
Show all issues
```
What if these fail?
```
usr/src/test/smbclient-tests/tests/configure/configure.ksh (Diff revision 1)
Show all issues
```
What if these fail?
```

usr/src/test/smbclient-tests/tests/configure/configure.ksh (Diff revision 1)

Show all issues

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?

gwr Feb. 28, 2018, 3:22 p.m.

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...

gwr April 10, 2018, 4:15 p.m.
```
Just removed it.
```

usr/src/test/smbclient-tests/tests/configure/configure.ksh (Diff revision 1)
Show all issues
```
Remove the XXXs?
```
1. gwr Feb. 28, 2018, 3:22 p.m.
  yeah, configure.ksh was all a "todo" I forgot about.
usr/src/test/smbclient-tests/tests/nsmbrc/tp_nsmbrc_002.ksh (Diff revision 1)
Show all issues
```
Check return value?
```
usr/src/test/smbclient-tests/tests/nsmbrc/tp_nsmbrc_002.ksh (Diff revision 1)
Show all issues
```
Check return value?
```
usr/src/test/smbclient-tests/tests/nsmbrc/tp_nsmbrc_002.ksh (Diff revision 1)
Show all issues
```
Check retun value?
```

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

Show all issues

Check return values / empty results?

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

Show all issues

Check return values / empty results?

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

Show all issues

Check return values / empty results?

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

Show all issues

Check return values / empty results?

usr/src/test/smbclient-tests/tests/smbfs/mmap/tp_mmap_001.ksh (Diff revision 1)

Show all issues

There's no copyright holder here? Can we maybe use the new cddl header for the tp_mmap_*.ksh tests?

usr/src/test/smbclient-tests/tests/smbfs/mmap/tp_mmap_005.ksh (Diff revision 1)
Show all issues
```
Trim trailing whitespace in this file?
```
usr/src/test/smbclient-tests/tests/smbfs/mmap/tp_mmap_009.ksh (Diff revision 1)
Show all issues
```
Can we at least make sure we have 80 columns in comments please?
```

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

Show all issues

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

gwr Feb. 24, 2018, 7:55 p.m.

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".

usr/src/test/test-runner/stf/contrib/include/ctiutils.shlib (Diff revision 1)
Show all issues
```
Any background on this XXX?
```
1. gwr Feb. 24, 2018, 7:55 p.m.
  I'm not sure. That was in the version Sun distributed.

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

Show all issues

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

gwr Feb. 24, 2018, 7:55 p.m.

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

gwr April 10, 2018, 4:15 p.m.
```
Just put that back as it was.
```

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

Show all issues

a heredoc may be easier to use here

gwr Feb. 24, 2018, 6:21 p.m.

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.

Change Summary:

Deliver a license for ctituils.shlib

Diff:

Revision 2 (+17835 -2)

Show changes

	usr/src/pkg/manifests/system-test-smbclient.mf
	usr/src/pkg/manifests/system-test-testrunner.mf
	usr/src/test/Makefile
	usr/src/test/smbclient-tests/Makefile.com
	usr/src/test/smbclient-tests/Makefile
	usr/src/test/smbclient-tests/cmd/Makefile.cmd
	usr/src/test/smbclient-tests/cmd/Makefile
	usr/src/test/smbclient-tests/cmd/abort_conn/Makefile
	175 more

Description:

		7519 Add smbclient tests

		Import the SMB client test from the old STC collection.
		Port those tests to STF.
		Add Jilin's mmap tests.
	+
	+	Many of the more recent tests need a bunch of cleanup,
	+	and I probably won't get to that for a while.
	+	I'll leave this open for comment, but reviewer time is
	+	probably better spent on my other reviewboards.

Change Summary:

Major update. This is the version we're using internaly now.
I hope we handled all the review feedback given previously.

Description:

		7519 Add smbclient tests

		Import the SMB client test from the old STC collection.
		Port those tests to STF.
~		Add Jilin's mmap tests.
~
	~	Add Jilin's mmap tests.
	~	Lots of cleanup.
-		Many of the more recent tests need a bunch of cleanup,
-		and I probably won't get to that for a while.
-		I'll leave this open for comment, but reviewer time is
-		probably better spent on my other reviewboards.

Testing Done:

~		run the new tests
	~	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)

Diff:

Revision 3 (+17440 -2)

Show changes

	usr/src/pkg/manifests/system-test-smbclient.mf
	usr/src/pkg/manifests/system-test-testrunner.mf
	usr/src/test/Makefile
	usr/src/test/smbclient-tests/Makefile.com
	usr/src/test/smbclient-tests/Makefile
	usr/src/test/smbclient-tests/cmd/Makefile.cmd
	usr/src/test/smbclient-tests/cmd/Makefile
	usr/src/test/smbclient-tests/cmd/abort_conn/Makefile
	180 more

usr/src/test/smbclient-tests/cmd/file_trunc/file_trunc.c (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/smbclient-tests/cmd/file_trunc/file_trunc.c (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/smbclient-tests/tests/Makefile (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/smbclient-tests/tests/nsmbrc/tp_nsmbrc_001.ksh (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/smbclient-tests/tests/sharectl/tp_sharectl_004.ksh (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/smbclient-tests/tests/smbfs/xattr/tp_xattr_003.ksh (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/smbclient-tests/tests/smbfs/xattr/tp_xattr_003.ksh (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/smbclient-tests/tests/smbfs/xattr/tp_xattr_003.ksh (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/smbclient-tests/tests/smbfs/xattr/tp_xattr_004.ksh (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/smbclient-tests/tests/smbfs/xattr/tp_xattr_005.ksh (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/smbclient-tests/tests/smbfs/xattr/tp_xattr_008.ksh (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/smbclient-tests/tests/smbutil/tp_smbutil_007.ksh (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/test-runner/stf/contrib/include/ctiutils.shlib (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/test-runner/stf/contrib/include/ctiutils.shlib (Diff revision 3)
Show all issues
```
red ugliness.
```
usr/src/test/test-runner/stf/contrib/include/ctiutils.shlib (Diff revision 3)
Show all issues
```
red ugliness.
```

Change Summary:

Fix space-tab sequences

Diff:

Revision 4 (+17440 -2)

Show changes

	usr/src/pkg/manifests/system-test-smbclient.mf
	usr/src/pkg/manifests/system-test-testrunner.mf
	usr/src/test/Makefile
	usr/src/test/smbclient-tests/Makefile.com
	usr/src/test/smbclient-tests/Makefile
	usr/src/test/smbclient-tests/cmd/Makefile.cmd
	usr/src/test/smbclient-tests/cmd/Makefile
	usr/src/test/smbclient-tests/cmd/abort_conn/Makefile
	180 more

Change Summary:

more ctyle etc

Diff:

Revision 5 (+17438 -2)

Show changes

	usr/src/pkg/manifests/system-test-smbclient.mf
	usr/src/pkg/manifests/system-test-testrunner.mf
	usr/src/test/Makefile
	usr/src/test/smbclient-tests/Makefile.com
	usr/src/test/smbclient-tests/Makefile
	usr/src/test/smbclient-tests/cmd/Makefile.cmd
	usr/src/test/smbclient-tests/cmd/Makefile
	usr/src/test/smbclient-tests/cmd/abort_conn/Makefile
	180 more

Change Summary:

more cstyle

Diff:

Revision 6 (+17438 -2)

Show changes

	usr/src/pkg/manifests/system-test-smbclient.mf
	usr/src/pkg/manifests/system-test-testrunner.mf
	usr/src/test/Makefile
	usr/src/test/smbclient-tests/Makefile.com
	usr/src/test/smbclient-tests/Makefile
	usr/src/test/smbclient-tests/cmd/Makefile.cmd
	usr/src/test/smbclient-tests/cmd/Makefile
	usr/src/test/smbclient-tests/cmd/abort_conn/Makefile
	180 more

Ship it!

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.

rm April 23, 2018, 3:42 p.m.

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.

gwr April 23, 2018, 9:04 p.m.

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.

usr/src/test/smbclient-tests/include/smbmount_common.ksh (Diff revisions 1 - 6)
Show all issues
```
Is there a reason this now requires sudo?
```
1. gwr April 23, 2018, 9:04 p.m.
  umount with -f required priv.
usr/src/test/smbclient-tests/tests/smbfs/acl/tp_acl_005.ksh (Diff revisions 1 - 6)
Show all issues
```
Another case where it seems sudo is now required?
```
1. gwr April 23, 2018, 9:04 p.m.
  chown required priv.

usr/src/test/smbclient-tests/tests/smbfs/xattr/tp_xattr_009.ksh (Diff revisions 1 - 6)

Could be $PWD, but doesn't matter.

gwr April 23, 2018, 9:04 p.m.

I thought I got rid of all the "cd somewhere; do things; cd back" logic,
so perhaps I should look at this one again.

gwr April 23, 2018, 9:10 p.m.

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

Change Summary:

Latest version of this, as shown on https://github.com/illumos/illumos-gate/pull/37/files

Diff:

Revision 7 (+17449 -2)

Show changes

	usr/src/pkg/manifests/system-test-smbclient.mf
	usr/src/pkg/manifests/system-test-testrunner.mf
	usr/src/test/Makefile
	usr/src/test/smbclient-tests/Makefile.com
	usr/src/test/smbclient-tests/Makefile
	usr/src/test/smbclient-tests/cmd/Makefile.cmd
	usr/src/test/smbclient-tests/cmd/Makefile
	usr/src/test/smbclient-tests/cmd/abort_conn/Makefile
	180 more

Change Summary:

Fix missing manifest entry, remove unused shell variable
I think this is RTI ready.

Diff:

Revision 8 (+17449 -2)

Show changes

	usr/src/pkg/manifests/system-test-smbclient.mf
	usr/src/pkg/manifests/system-test-testrunner.mf
	usr/src/test/Makefile
	usr/src/test/smbclient-tests/Makefile.com
	usr/src/test/smbclient-tests/Makefile
	usr/src/test/smbclient-tests/cmd/Makefile.cmd
	usr/src/test/smbclient-tests/cmd/Makefile
	usr/src/test/smbclient-tests/cmd/abort_conn/Makefile
	180 more

You have a pending review.

Review Board 3.0.18

7519 Add smbclient tests

Change Summary:

Diff:

Description:

Change Summary:

Description:

Testing Done:

Diff:

Change Summary:

Diff:

Change Summary:

Diff:

Change Summary:

Diff:

Change Summary:

Diff:

Change Summary:

Diff:

Status: Closed (submitted)