From f79bde1a6d6899641370835fb2aafcd8ec224301 Mon Sep 17 00:00:00 2001
From: Paul Smith <psmith@gnu.org>
Date: Wed, 1 Apr 2020 01:58:33 -0400
Subject: [PATCH] tests: Simplify customization of %ENV

Rather than having an %extraENV that is added to the default %ENV
and resetting %ENV _before_ each test, allow the test setup to
modify %ENV directly as needed then reset %ENV _after_ each test.

* tests/test_driver.pl: Remove unused %extraENV.
(resetENV): Don't add in %extraENV.
(_run_command): Reset after we run the command rather than before.
* tests/scripts/features/export: Convert %extraENV to %ENV
* tests/scripts/features/jobserver: Ditto
* tests/scripts/features/parallelism: Ditto
* tests/scripts/features/targetvars: Ditto
* tests/scripts/functions/eval: Ditto
* tests/scripts/functions/foreach: Ditto
* tests/scripts/functions/origin: Ditto
* tests/scripts/misc/general4: Ditto
* tests/scripts/options/dash-e: Ditto
* tests/scripts/targets/POSIX: Ditto
* tests/scripts/variables/GNUMAKEFLAGS: Ditto
* tests/scripts/variables/SHELL: Ditto
---
 tests/scripts/features/export        | 20 ++++++++++++--------
 tests/scripts/features/jobserver     |  6 ++----
 tests/scripts/features/parallelism   |  3 +--
 tests/scripts/features/targetvars    |  2 +-
 tests/scripts/functions/eval         |  2 +-
 tests/scripts/functions/foreach      |  4 +---
 tests/scripts/functions/origin       | 28 +++++++++++++---------------
 tests/scripts/misc/general4          |  2 +-
 tests/scripts/options/dash-e         | 17 ++++-------------
 tests/scripts/targets/POSIX          |  2 +-
 tests/scripts/variables/GNUMAKEFLAGS |  6 +++---
 tests/scripts/variables/SHELL        | 11 +++++------
 tests/test_driver.pl                 | 19 ++++---------------
 13 files changed, 49 insertions(+), 73 deletions(-)

diff --git a/tests/scripts/features/export b/tests/scripts/features/export
index 81bff0c3..0b07abb9 100644
--- a/tests/scripts/features/export
+++ b/tests/scripts/features/export
@@ -32,16 +32,17 @@ ifdef EXPORT_ALL_PSEUDO
 .EXPORT_ALL_VARIABLES:
 endif
 
+.RECIPEPREFIX := >
 all:
-	@echo "FOO=$(FOO) BAR=$(BAR) BAZ=$(BAZ) BOZ=$(BOZ) BITZ=$(BITZ) BOTZ=$(BOTZ)"
-	@echo "FOO=$$FOO BAR=$$BAR BAZ=$$BAZ BOZ=$$BOZ BITZ=$$BITZ BOTZ=$$BOTZ"
+> @echo "FOO=$(FOO) BAR=$(BAR) BAZ=$(BAZ) BOZ=$(BOZ) BITZ=$(BITZ) BOTZ=$(BOTZ)"
+> @echo "FOO=$$FOO BAR=$$BAR BAZ=$$BAZ BOZ=$$BOZ BITZ=$$BITZ BOTZ=$$BOTZ"
 ',
            '', "FOO=foo BAR=bar BAZ=baz BOZ=boz BITZ=bitz BOTZ=botz
 FOO= BAR= BAZ=baz BOZ=boz BITZ=bitz BOTZ=\n");
 
 # TEST 1: make sure vars inherited from the parent are exported
 
-$extraENV{FOO} = 1;
+$ENV{FOO} = 1;
 
 &run_make_test(undef, '', "FOO=foo BAR=bar BAZ=baz BOZ=boz BITZ=bitz BOTZ=botz
 FOO=foo BAR= BAZ=baz BOZ=boz BITZ=bitz BOTZ=\n");
@@ -85,9 +86,10 @@ B = b
 export $(FOO)
 export $(B)ar
 
+.RECIPEPREFIX := >
 all:
-	@echo foo=$(foo) bar=$(bar)
-	@echo foo=$$foo bar=$$bar
+> @echo foo=$(foo) bar=$(bar)
+> @echo foo=$$foo bar=$$bar
 ',
              "", "foo=f-ok bar=b-ok\nfoo=f-ok bar=b-ok\n");
 
@@ -108,9 +110,10 @@ export foo bar
 unexport $(FOO)
 unexport $(B)ar
 
+.RECIPEPREFIX := >
 all:
-	@echo foo=$(foo) bar=$(bar)
-	@echo foo=$$foo bar=$$bar
+> @echo foo=$(foo) bar=$(bar)
+> @echo foo=$$foo bar=$$bar
 ',
               '', "foo=f-ok bar=b-ok\nfoo= bar=\n");
 
@@ -140,7 +143,8 @@ all: ; @echo A=$$A B=$$B C=$$C D=$$D E=$$E F=$$F G=$$G H=$$H I=$$I J=$$J
 
 # TEST 8: Test unexporting multiple variables on the same line
 
-@extraENV{qw(A B C D E F G H I J)} = qw(1 2 3 4 5 6 7 8 9 10);
+@args{qw(A B C D E F G H I J)} = qw(1 2 3 4 5 6 7 8 9 10);
+%ENV = (%ENV, %args);
 
 &run_make_test('
 A = a
diff --git a/tests/scripts/features/jobserver b/tests/scripts/features/jobserver
index 2d61e3f4..35b56962 100644
--- a/tests/scripts/features/jobserver
+++ b/tests/scripts/features/jobserver
@@ -25,17 +25,16 @@ all:;@echo $@: "/$(SHOW)/"
 
 # Setting parallelism with the environment
 # Command line should take precedence over the environment
-$extraENV{MAKEFLAGS} = "-j2 $np";
+$ENV{MAKEFLAGS} = "-j2 $np";
 run_make_test(q!
 SHOW = $(patsubst --jobserver-auth=%,--jobserver-auth=<auth>,$(MAKEFLAGS))
 recurse: ; @echo $@: "/$(SHOW)/"; $(MAKE) -f #MAKEFILE# all
 all:;@echo $@: "/$(SHOW)/"
 !,
               '', "recurse: /-j2 --jobserver-auth=<auth> $np/\nall: /-j2 --jobserver-auth=<auth> $np/\n");
-delete $extraENV{MAKEFLAGS};
 
 # Test override of -jN
-$extraENV{MAKEFLAGS} = "-j9 $np";
+$ENV{MAKEFLAGS} = "-j9 $np";
 run_make_test(q!
 SHOW = $(patsubst --jobserver-auth=%,--jobserver-auth=<auth>,$(MAKEFLAGS))
 recurse: ; @echo $@: "/$(SHOW)/"; $(MAKE) -j3 -f #MAKEFILE# recurse2
@@ -43,7 +42,6 @@ recurse2: ; @echo $@: "/$(SHOW)/"; $(MAKE) -f #MAKEFILE# all
 all:;@echo $@: "/$(SHOW)/"
 !,
               "-j2 $np", "recurse: /-j2 --jobserver-auth=<auth> $np/\n#MAKE#[1]: warning: -j3 forced in submake: resetting jobserver mode.\nrecurse2: /-j3 --jobserver-auth=<auth> $np/\nall: /-j3 --jobserver-auth=<auth> $np/\n");
-delete $extraENV{MAKEFLAGS};
 
 # Test override of -jN with -j
 run_make_test(q!
diff --git a/tests/scripts/features/parallelism b/tests/scripts/features/parallelism
index 08f94a6b..e9935b4b 100644
--- a/tests/scripts/features/parallelism
+++ b/tests/scripts/features/parallelism
@@ -148,7 +148,7 @@ rmfiles('target');
 # TEST #11: Make sure -jN from MAKEFLAGS is processed even when we re-exec
 # See Savannah bug #33873
 
-$extraENV{MAKEFLAGS} = '-j4';
+$ENV{MAKEFLAGS} = '-j4';
 
 run_make_test(q!
 things = thing1 thing2
@@ -160,7 +160,6 @@ inc.mk: ; @touch $@
 !,
               '', "file thing2start\nwait thing2start\nfile thing1start\nwait thing1start\nfile thing2end\nwait thing2end\nthing1end\n");
 
-delete $extraENV{MAKEFLAGS};
 rmfiles(qw(inc.mk thing1start thing1end thing2start thing2end));
 
 # Ensure intermediate/secondary files are not pruned incorrectly.
diff --git a/tests/scripts/features/targetvars b/tests/scripts/features/targetvars
index 2929b2cc..edc90b81 100644
--- a/tests/scripts/features/targetvars
+++ b/tests/scripts/features/targetvars
@@ -270,7 +270,7 @@ all: ; $sname >/dev/null
 
 # Don't use the general PATH if not found on the target path
 
-$extraENV{PATH} = "$ENV{PATH}:sd";
+$ENV{PATH} = "$ENV{PATH}:sd";
 
 run_make_test(qq!
 all: PATH := ..
diff --git a/tests/scripts/functions/eval b/tests/scripts/functions/eval
index 90513bdd..a3fba87a 100644
--- a/tests/scripts/functions/eval
+++ b/tests/scripts/functions/eval
@@ -127,7 +127,7 @@ EOF
 
 close(MAKEFILE);
 
-$extraENV{EVAR} = '1';
+$ENV{EVAR} = '1';
 &run_make_with_options($makefile4, "", &get_logfile);
 $answer = "OK\n";
 &compare_output($answer,&get_logfile(1));
diff --git a/tests/scripts/functions/foreach b/tests/scripts/functions/foreach
index 9badc52e..6a06109a 100644
--- a/tests/scripts/functions/foreach
+++ b/tests/scripts/functions/foreach
@@ -14,7 +14,7 @@ types of foreach loops are tested\n";
 # TEST 0
 
 # Set an environment variable that we can test in the makefile.
-$extraENV{FOOFOO} = 'foo foo';
+$ENV{FOOFOO} = 'foo foo';
 
 run_make_test("space = ' '".'
 null :=
@@ -31,8 +31,6 @@ for2: ; @echo $(fe)',
               "undefined file default file environment default file command line override automatic automatic
 foo.o bletch.o null.o @.o garf.o .o    .o undefined.o file.o default.o file.o environment.o default.o file.o command.o line.o override.o automatic.o automatic.o");
 
-delete $extraENV{FOOFOO};
-
 # TEST 1: Test that foreach variables take precedence over global
 # variables in a global scope (like inside an eval).  Tests bug #11913
 
diff --git a/tests/scripts/functions/origin b/tests/scripts/functions/origin
index f7b7eb84..9b9fd56a 100644
--- a/tests/scripts/functions/origin
+++ b/tests/scripts/functions/origin
@@ -16,26 +16,27 @@ defined per the following list:
 'automatic'            Automatic variable\n";
 
 # Set an environment variable
-$extraENV{MAKETEST} = 1;
+$ENV{MAKETEST} = 1;
 
 run_make_test('
 foo := bletch garf
 auto_var = undefined CC MAKETEST MAKE foo CFLAGS WHITE @
 av = $(foreach var, $(auto_var), $(origin $(var)) )
 override WHITE := BLACK
+.RECIPEPREFIX = >
 all: auto
-	@echo $(origin undefined)
-	@echo $(origin CC)
-	@echo $(origin MAKETEST)
-	@echo $(origin MAKE)
-	@echo $(origin foo)
-	@echo $(origin CFLAGS)
-	@echo $(origin WHITE)
-	@echo $(origin @)
+> @echo $(origin undefined)
+> @echo $(origin CC)
+> @echo $(origin MAKETEST)
+> @echo $(origin MAKE)
+> @echo $(origin foo)
+> @echo $(origin CFLAGS)
+> @echo $(origin WHITE)
+> @echo $(origin @)
 auto :
-	@echo $(av)',
-	      '-e WHITE=WHITE CFLAGS=',
-	      'undefined default environment default file command line override automatic
+> @echo $(av)',
+    '-e WHITE=WHITE CFLAGS=',
+    'undefined default environment default file command line override automatic
 undefined
 default
 environment
@@ -45,7 +46,4 @@ command line
 override
 automatic');
 
-# Reset an environment variable
-delete $extraENV{MAKETEST};
-
 1;
diff --git a/tests/scripts/misc/general4 b/tests/scripts/misc/general4
index eeb8d106..263505e4 100644
--- a/tests/scripts/misc/general4
+++ b/tests/scripts/misc/general4
@@ -94,7 +94,7 @@ all: ; $sname >/dev/null
 
 # Don't use the general PATH if not found on the target path
 
-$extraENV{PATH} = "$ENV{PATH}:sd";
+$ENV{PATH} = "$ENV{PATH}:sd";
 
 run_make_test(qq!
 PATH := ..
diff --git a/tests/scripts/options/dash-e b/tests/scripts/options/dash-e
index 17c3fc86..944c39df 100644
--- a/tests/scripts/options/dash-e
+++ b/tests/scripts/options/dash-e
@@ -4,21 +4,12 @@ $description = "The following test creates a makefile to ...";
 
 $details = "";
 
-$extraENV{GOOGLE} = 'boggle';
+$ENV{GOOGLE} = 'boggle';
 
-open(MAKEFILE,"> $makefile");
-
-print MAKEFILE <<'EOF';
+run_make_test(q!
 GOOGLE = bazzle
 all:; @echo "$(GOOGLE)"
-EOF
-
-close(MAKEFILE);
-
-&run_make_with_options($makefile, '-e' ,&get_logfile);
-
-$answer = "boggle\n";
-
-&compare_output($answer,&get_logfile(1));
+!,
+              '-e', "boggle\n");
 
 1;
diff --git a/tests/scripts/targets/POSIX b/tests/scripts/targets/POSIX
index c44cc500..d69b8bbd 100644
--- a/tests/scripts/targets/POSIX
+++ b/tests/scripts/targets/POSIX
@@ -47,7 +47,7 @@ $make
               '', $r);
 
 # Make sure that local settings take precedence
-%extraENV = map { $_ => "xx-$_" } keys %POSIX;
+%ENV = (%ENV, map { $_ => "xx-$_" } keys %POSIX);
 $r = join('', map { "$_=xx-$_\n"} sort keys %POSIX);
 run_make_test(undef, '', $r);
 
diff --git a/tests/scripts/variables/GNUMAKEFLAGS b/tests/scripts/variables/GNUMAKEFLAGS
index 62a68bd9..063c256d 100644
--- a/tests/scripts/variables/GNUMAKEFLAGS
+++ b/tests/scripts/variables/GNUMAKEFLAGS
@@ -5,7 +5,7 @@ $description = "Test proper behavior of GNUMAKEFLAGS";
 # Accept flags from GNUMAKEFLAGS as well as MAKEFLAGS
 # Results always go in MAKEFLAGS
 
-$extraENV{'GNUMAKEFLAGS'} = '-e -r -R';
+$ENV{'GNUMAKEFLAGS'} = '-e -r -R';
 
 run_make_test(q!
 all: ; @echo $(MAKEFLAGS)
@@ -14,7 +14,7 @@ all: ; @echo $(MAKEFLAGS)
 
 # Long arguments mean everything is prefixed with "-"
 
-$extraENV{'GNUMAKEFLAGS'} = '--no-print-directory -e -r -R --trace';
+$ENV{'GNUMAKEFLAGS'} = '--no-print-directory -e -r -R --trace';
 
 run_make_test(q!
 all: ; @echo $(MAKEFLAGS)
@@ -27,7 +27,7 @@ erR --trace --no-print-directory");
 
 unlink('x.mk');
 
-$extraENV{GNUMAKEFLAGS} = '-Itst/bad';
+$ENV{GNUMAKEFLAGS} = '-Itst/bad';
 
 run_make_test(q!
 recurse: ; @echo $@; echo MAKEFLAGS = $$MAKEFLAGS; echo GNUMAKEFLAGS = $$GNUMAKEFLAGS; #MAKEPATH# -f #MAKEFILE# all
diff --git a/tests/scripts/variables/SHELL b/tests/scripts/variables/SHELL
index 4ff1ac25..34e72be4 100644
--- a/tests/scripts/variables/SHELL
+++ b/tests/scripts/variables/SHELL
@@ -14,9 +14,8 @@ $mshell = $sh_name;
 
 # According to POSIX, the value of SHELL in the environment has no impact on
 # the value in the makefile.
-# Note %extraENV takes precedence over the default value for the shell.
 
-$extraENV{SHELL} = '/dev/null';
+$ENV{SHELL} = '/dev/null';
 run_make_test('all:;@echo "$(SHELL)"', '', $mshell);
 
 # According to POSIX, any value of SHELL set in the makefile should _NOT_ be
@@ -25,7 +24,7 @@ run_make_test('all:;@echo "$(SHELL)"', '', $mshell);
 # all when $(SHELL) is perl :-/.  So, we just add an extra initial /./ which
 # works well on UNIX and seems to work OK on at least some non-UNIX systems.
 
-$extraENV{SHELL} = $mshell;
+$ENV{SHELL} = $mshell;
 
 my $altshell = "/./$mshell";
 my $altshell2 = "/././$mshell";
@@ -41,7 +40,7 @@ all:;@echo "$(SHELL) $$SHELL"
 # As a GNU make extension, if make's SHELL variable is explicitly exported,
 # then we really _DO_ export it.
 
-$extraENV{SHELL} = $mshell;
+$ENV{SHELL} = $mshell;
 
 run_make_test("export SHELL := $altshell\n".'
 all:;@echo "$(SHELL) $$SHELL"
@@ -51,13 +50,13 @@ all:;@echo "$(SHELL) $$SHELL"
 # Test out setting of SHELL, both exported and not, as a target-specific
 # variable.
 
-$extraENV{SHELL} = $mshell;
+$ENV{SHELL} = $mshell;
 
 run_make_test("all: SHELL := $altshell\n".'
 all:;@echo "$(SHELL) $$SHELL"
 ', '', "$altshell $mshell");
 
-$extraENV{SHELL} = $mshell;
+$ENV{SHELL} = $mshell;
 
 run_make_test("
 SHELL := $altshell2
diff --git a/tests/test_driver.pl b/tests/test_driver.pl
index ccd43c5f..ec17be1f 100644
--- a/tests/test_driver.pl
+++ b/tests/test_driver.pl
@@ -83,13 +83,9 @@ if ($^O ne 'VMS') {
 # in recipes
 $perl_name =~ tr,\\,/,;
 
-# %makeENV is the cleaned-out environment.
+# %makeENV is the cleaned-out environment.  Tests must not modify it.
 %makeENV = ();
 
-# %extraENV are any extra environment variables the tests might want to set.
-# These are RESET AFTER EVERY TEST!
-%extraENV = ();
-
 sub vms_get_process_logicals {
   # Sorry for the long note here, but to keep this test running on
   # VMS, it is needed to be understood.
@@ -150,11 +146,6 @@ sub resetENV
       }
     }
   }
-
-  foreach $v (keys %extraENV) {
-    $ENV{$v} = $extraENV{$v};
-    delete $extraENV{$v};
-  }
 }
 
 sub toplevel
@@ -1059,15 +1050,13 @@ sub _run_with_timeout
 # This runs a command without any debugging info.
 sub _run_command
 {
-  # We reset this before every invocation.  On Windows I think there is only
-  # one environment, not one per process, so I think that variables set in
-  # test scripts might leak into subsequent tests if this isn't reset--???
-  resetENV();
-
   my $orig = $SIG{ALRM};
   my $code = eval { _run_with_timeout(@_); };
   $SIG{ALRM} = $orig;
 
+  # Reset then environment so that it's clean for the next test.
+  resetENV();
+
   if ($@) {
     # The eval failed.  If it wasn't SIGALRM then die.
     $@ eq "timeout\n" or die "Command failed: $@";