sdimitro approved this pull request.

Can you add an entry in `usr/src/pkg/manifests/system-test-zfstest.mf` for the 
new test that you added?

Things look good to me overall since I've had a sneak peek of the ZoL review. 
I'd have @jwk404 take a look at the new test case though just in case.

> +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or http://www.opensolaris.org/os/licensing.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, usr/src/common/zfs this CDDL HEADER in each
+# file and usr/src/common/zfs the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright 2007 Sun Microsystems, Inc.  All rights reserved.

Copy paste error?

> +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or http://www.opensolaris.org/os/licensing.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, usr/src/common/zfs this CDDL HEADER in each
+# file and usr/src/common/zfs the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright 2007 Sun Microsystems, Inc.  All rights reserved.

Copy-paste error?

> +# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy is of the CDDL is also available via the Internet
+# at http://www.illumos.org/license/CDDL.
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2018 Datto Inc.
+#
+
+. $STF_SUITE/usr/src/common/zfs/libtest.shlib

why `$STF_SUITE/usr/src/common/zfs/libtest.shlib`?
and not `. $STF_SUITE/include/libtest.shlib`?

I saw your makefiles doing that too. Could you maybe change the Makefiles and 
the include in that file to be consistent with the rest of the stuff under 
`cli_root`?

> +"{
+    \"return\": {
+        \"failed\": {},
+        \"succeeded\": {
+            \"$TESTDS\": \"filesystem\"
+        }
+    }
+}")
+typeset -i cnt=0
+typeset usr/src/cmd
+for usr/src/cmd in ${pos_usr/src/cmds[@]}; do
+       log_must zfs program $TESTPOOL $TESTZCP $TESTDS $usr/src/cmd 2>&1
+       log_must zfs program $TESTPOOL -j $TESTZCP $TESTDS $usr/src/cmd 2>&1
+       # json.tool is needed to guarantee consistent ordering of fields
+       # sed is needed to trim trailing space in CentOS 6's json.tool output
+       OUTPUT=$(zfs program $TESTPOOL -j $TESTZCP $TESTDS $usr/src/cmd 2>&1 | 
python -m json.tool | sed 's/[[:space:]]*$//')

[nit] Not sure if we do style checks for shell scripts too but can you maybe 
wrap this line?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/619#pullrequestreview-112048253
------------------------------------------
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/discussions/T0465226805877059-Mf71c8e37ec32e2b89c1f82d0
Delivery options: https://openzfs.topicbox.com/groups

Reply via email to