Always preserve the environment for specs that modify ENV

spec/compiler/macro/macro_methods_spec.cr

@@ -1,4 +1,5 @@
 require "../../spec_helper"
+require "../../support/env"
 
 private def declare_class_var(container : ClassVarContainer, name, var_type : Type, annotations = nil)
   var = MetaTypeVar.new(name)
@@ -3281,14 +3282,15 @@ module Crystal
 
     describe "env" do
       it "has key" do
-        ENV["FOO"] = "foo"
-        assert_macro %({{env("FOO")}}), %("foo")
-        ENV.delete "FOO"
+        with_env("FOO": "foo") do
+          assert_macro %({{env("FOO")}}), %("foo")
+        end
       end
 
       it "doesn't have key" do
-        ENV.delete "FOO"
-        assert_macro %({{env("FOO")}}), %(nil)
+        with_env("FOO": nil) do
+          assert_macro %({{env("FOO")}}), %(nil)
+        end
       end
     end
 

spec/std/dir_spec.cr

@@ -3,25 +3,9 @@ require "../support/env"
 
 private def unset_tempdir(&)
   {% if flag?(:windows) %}
-    old_tempdirs = {ENV["TMP"]?, ENV["TEMP"]?, ENV["USERPROFILE"]?}
-    begin
-      ENV.delete("TMP")
-      ENV.delete("TEMP")
-      ENV.delete("USERPROFILE")
-
-      yield
-    ensure
-      ENV["TMP"], ENV["TEMP"], ENV["USERPROFILE"] = old_tempdirs
-    end
+    with_env("TMP": nil, "TEMP": nil, "USERPROFILE": nil) { yield }
   {% else %}
-    begin
-      old_tempdir = ENV["TMPDIR"]?
-      ENV.delete("TMPDIR")
-
-      yield
-    ensure
-      ENV["TMPDIR"] = old_tempdir
-    end
+    with_env("TMPDIR": nil) { yield }
   {% end %}
 end
 

spec/std/env_spec.cr

@@ -2,6 +2,23 @@ require "spec"
 require "./spec_helper"
 
 describe "ENV" do
+  # Preserves the existing environment for each spec.
+  # To avoid potential circular definitions, this has to use the system methods
+  # directly, rather than `ENV` or `with_env`.
+  around_each do |example|
+    old_env = {} of String => String
+    Crystal::System::Env.each { |key, value| old_env[key] = value }
+
+    begin
+      example.run
+    ensure
+      keys = [] of String
+      Crystal::System::Env.each { |key| keys << key }
+      keys.each { |key| Crystal::System::Env.set(key, nil) }
+      old_env.each { |key, value| Crystal::System::Env.set(key, value) }
+    end
+  end
+
   it "gets non existent key raises" do
     expect_raises KeyError, "Missing ENV key: \"NON-EXISTENT\"" do
       ENV["NON-EXISTENT"]
@@ -16,24 +33,18 @@ describe "ENV" do
     (ENV["FOO"] = "1").should eq("1")
     ENV["FOO"].should eq("1")
     ENV["FOO"]?.should eq("1")
-  ensure
-    ENV.delete("FOO")
   end
 
   {% if flag?(:win32) %}
     it "sets and gets case-insensitive" do
       (ENV["FOO"] = "1").should eq("1")
       ENV["Foo"].should eq("1")
       ENV["foo"]?.should eq("1")
-    ensure
-      ENV.delete("FOO")
     end
   {% else %}
     it "sets and gets case-sensitive" do
       ENV["FOO"] = "1"
       ENV["foo"]?.should be_nil
-    ensure
-      ENV.delete("FOO")
     end
   {% end %}
 
@@ -47,16 +58,12 @@ describe "ENV" do
   it "sets to empty string" do
     (ENV["FOO_EMPTY"] = "").should eq ""
     ENV["FOO_EMPTY"]?.should eq ""
-  ensure
-    ENV.delete("FOO_EMPTY")
   end
 
   it "does has_key?" do
     ENV["FOO"] = "1"
     ENV.has_key?("NON_EXISTENT").should be_false
     ENV.has_key?("FOO").should be_true
-  ensure
-    ENV.delete("FOO")
   end
 
   it "deletes a key" do
@@ -70,9 +77,6 @@ describe "ENV" do
     %w(FOO BAR).each { |k| ENV.keys.should_not contain(k) }
     ENV["FOO"] = ENV["BAR"] = "1"
     %w(FOO BAR).each { |k| ENV.keys.should contain(k) }
-  ensure
-    ENV.delete("FOO")
-    ENV.delete("BAR")
   end
 
   it "does not have an empty key" do
@@ -86,9 +90,6 @@ describe "ENV" do
     ENV["FOO"] = "SOMEVALUE_1"
     ENV["BAR"] = "SOMEVALUE_2"
     [1, 2].each { |i| ENV.values.should contain("SOMEVALUE_#{i}") }
-  ensure
-    ENV.delete("FOO")
-    ENV.delete("BAR")
   end
 
   describe "[]=" do
@@ -115,34 +116,26 @@ describe "ENV" do
     it "fetches with one argument" do
       ENV["1"] = "2"
       ENV.fetch("1").should eq("2")
-    ensure
-      ENV.delete("1")
     end
 
     it "fetches with default value" do
       ENV["1"] = "2"
       ENV.fetch("1", "3").should eq("2")
       ENV.fetch("2", "3").should eq("3")
-    ensure
-      ENV.delete("1")
     end
 
     it "fetches with block" do
       ENV["1"] = "2"
       ENV.fetch("1") { |k| k + "block" }.should eq("2")
       ENV.fetch("2") { |k| k + "block" }.should eq("2block")
       ENV.fetch("3") { 4 }.should eq(4)
-    ensure
-      ENV.delete("1")
     end
 
     it "fetches and raises" do
       ENV["1"] = "2"
       expect_raises KeyError, "Missing ENV key: \"2\"" do
         ENV.fetch("2")
       end
-    ensure
-      ENV.delete("1")
     end
   end
 
@@ -162,16 +155,11 @@ describe "ENV" do
       "TEST_UNICODE_1" => "bar\u{d7ff}\u{10000}",
       "TEST_UNICODE_2" => "\u{1234}",
     })
-  ensure
-    ENV.delete("TEST_UNICODE_1")
-    ENV.delete("TEST_UNICODE_2")
   end
 
   it "#to_h" do
     ENV["FOO"] = "foo"
     ENV.to_h["FOO"].should eq "foo"
-  ensure
-    ENV.delete("FOO")
   end
 
   {% if flag?(:win32) %}

spec/std/process_spec.cr

@@ -3,6 +3,7 @@
 require "spec"
 require "process"
 require "./spec_helper"
+require "../support/env"
 
 private def exit_code_command(code)
   {% if flag?(:win32) %}
@@ -267,68 +268,62 @@ describe Process do
       end
 
       it "deletes existing environment variable" do
-        ENV["FOO"] = "bar"
-        value = Process.run(*print_env_command, env: {"FOO" => nil}) do |proc|
-          proc.output.gets_to_end
+        with_env("FOO": "bar") do
+          value = Process.run(*print_env_command, env: {"FOO" => nil}) do |proc|
+            proc.output.gets_to_end
+          end
+          value.should_not match /(*ANYCRLF)^FOO=/m
         end
-        value.should_not match /(*ANYCRLF)^FOO=/m
-      ensure
-        ENV.delete("FOO")
       end
 
       {% if flag?(:win32) %}
         it "deletes existing environment variable case-insensitive" do
-          ENV["FOO"] = "bar"
-          value = Process.run(*print_env_command, env: {"foo" => nil}) do |proc|
-            proc.output.gets_to_end
+          with_env("FOO": "bar") do
+            value = Process.run(*print_env_command, env: {"foo" => nil}) do |proc|
+              proc.output.gets_to_end
+            end
+            value.should_not match /(*ANYCRLF)^FOO=/mi
           end
-          value.should_not match /(*ANYCRLF)^FOO=/mi
-        ensure
-          ENV.delete("FOO")
         end
       {% end %}
 
       it "preserves existing environment variable" do
-        ENV["FOO"] = "bar"
-        value = Process.run(*print_env_command) do |proc|
-          proc.output.gets_to_end
+        with_env("FOO": "bar") do
+          value = Process.run(*print_env_command) do |proc|
+            proc.output.gets_to_end
+          end
+          value.should match /(*ANYCRLF)^FOO=bar$/m
         end
-        value.should match /(*ANYCRLF)^FOO=bar$/m
-      ensure
-        ENV.delete("FOO")
       end
 
       it "preserves and sets an environment variable" do
-        ENV["FOO"] = "bar"
-        value = Process.run(*print_env_command, env: {"FOO2" => "bar2"}) do |proc|
-          proc.output.gets_to_end
+        with_env("FOO": "bar") do
+          value = Process.run(*print_env_command, env: {"FOO2" => "bar2"}) do |proc|
+            proc.output.gets_to_end
+          end
+          value.should match /(*ANYCRLF)^FOO=bar$/m
+          value.should match /(*ANYCRLF)^FOO2=bar2$/m
         end
-        value.should match /(*ANYCRLF)^FOO=bar$/m
-        value.should match /(*ANYCRLF)^FOO2=bar2$/m
-      ensure
-        ENV.delete("FOO")
       end
 
       it "overrides existing environment variable" do
-        ENV["FOO"] = "bar"
-        value = Process.run(*print_env_command, env: {"FOO" => "different"}) do |proc|
-          proc.output.gets_to_end
+        with_env("FOO": "bar") do
+          value = Process.run(*print_env_command, env: {"FOO" => "different"}) do |proc|
+            proc.output.gets_to_end
+          end
+          value.should match /(*ANYCRLF)^FOO=different$/m
         end
-        value.should match /(*ANYCRLF)^FOO=different$/m
-      ensure
-        ENV.delete("FOO")
       end
 
       {% if flag?(:win32) %}
         it "overrides existing environment variable case-insensitive" do
-          ENV["FOO"] = "bar"
-          value = Process.run(*print_env_command, env: {"fOo" => "different"}) do |proc|
-            proc.output.gets_to_end
+          with_env("FOO": "bar") do
+            value = Process.run(*print_env_command, env: {"fOo" => "different"}) do |proc|
+              proc.output.gets_to_end
+            end
+            value.should_not match /(*ANYCRLF)^FOO=/m
+            value.should match /(*ANYCRLF)^fOo=different$/m
           end
-          value.should_not match /(*ANYCRLF)^FOO=/m
-          value.should match /(*ANYCRLF)^fOo=different$/m
-        ensure
-          ENV.delete("FOO")
         end
       {% end %}
     end