git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] tests: turn on test-lint-shell-syntax by default

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 27, 2013, 17:34 UTC
Message-ID
<7v4ni2y1fm.fsf@alter.siamese.dyndns.org>
In-Reply-To
<5105280A.80002@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> Back to the which:
> ...
> and running "make test" gives the following, at least in my system:
> ...

I think everybody involved in this discussion already knows that; the point is that it can easily give false negative, without the scripts working very hard to do so.

If we did not care about incurring runtime performance cost, we could arrange:

 - the test framework to define a variable $TEST_ABORT that has a
   full path to a file that is in somewhere test authors cannot
   touch unless they really try hard to (i.e. preferrably outside
   $TRASH_DIRECTORY, as it is not uncommon for to tests to do "rm *"
   there). This location should be per $(basename "$0" .sh) to allow
   running multiple tests in paralell;
 - the test framework to "rm -f $TEST_ABORT" at the beginning of
   test_expect_success/failure;
 - test_expect_success/failure to check $TEST_ABORT and if it
   exists, abort the execution, showing the contents of the file as
   an error message.

Then you can wrap commands whose use we want to limit, perhaps like this, in the test framework:

	which () {
		cat >"$TEST_ABORT" <<-\EOF
		Do not use unportable 'which' in the test script.
                "if type $cmd" is a good way to see if $cmd exists.
		EOF
	}
	sed () {
		saw_wantarg= must_abort=
                for arg
                do
			if test -n "$saw_wantarg"
                        then
				saw_wantarg=
                                continue
			fi
			case "$arg" in
			--)	break ;; # end of options
			-i)	echo >"$TEST_ABORT" "Do not use 'sed -i'"
				must_abort=yes
				break ;;
                        -e)	saw_wantarg=yes ;; # skip next arg
			-*)	continue ;; # options without arg
			*)	break ;; # filename
			esac
		done
		if test -z "$must_abort"
			sed "$@"
		fi
	}

Then you can check that TEST_ABORT does not appear in test scripts (ensuring that they do not attempt to circumvent the mechanis) and catch use of unwanted commands or unwanted extended features of commands at runtime.

But this will incur runtime performace hit, so I am not sure it would be worth it.

Previous: Torsten BögershausenNext: Junio C Hamano
Message 14 of 19 in “tests: turn on test-lint-shell-syntax by default”
  1. tests: turn on test-lint-shell-syntax by defaultTorsten Bögershausen, Jan 12, 2013
  2. Junio C HamanoJan 12, 2013
  3. Torsten BögershausenJan 13, 2013
  4. Matt KraaiJan 13, 2013
  5. Jonathan NiederJan 13, 2013
  6. Junio C HamanoJan 13, 2013
  7. Torsten BögershausenJan 15, 2013
  8. Junio C HamanoJan 15, 2013
  9. Torsten BögershausenJan 26, 2013
  10. Junio C HamanoJan 26, 2013
  11. Torsten BögershausenJan 27, 2013
  12. Jonathan NiederJan 27, 2013
  13. Torsten BögershausenJan 27, 2013
  14. Junio C HamanoJan 27, 2013
  15. Junio C HamanoJan 27, 2013
  16. Torsten BögershausenFeb 5, 2013
  17. Junio C HamanoFeb 5, 2013
  18. Junio C HamanoFeb 5, 2013
  19. Junio C HamanoJan 27, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.