Ensure tests don't leave artifacts in dev environment (#701)

* Fix byebug in test context 

- Send deprecation warnings to log in test, so rewrite of stderr hopefully not needed

* Clean up test setup, ensure it doesn't leave files

* Ensure the test environment vars are isolated from dev environment vars

* Use around block only for environment variables

spec/controllers/omniauth_controller_spec.rb

@@ -10,15 +10,15 @@
     end
 
     context 'not using database auth' do
-      cached_database_auth = ENV['DATABASE_AUTH']
-
-      before do
+      around do |example|
+        cached_database_auth = ENV['DATABASE_AUTH']
         ENV['DATABASE_AUTH'] = 'false'
-        allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new("production"))
+        example.run
+        ENV['DATABASE_AUTH'] = cached_database_auth
       end
 
-      after do
-        ENV['DATABASE_AUTH'] = cached_database_auth
+      before do
+        allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new("production"))
       end
 
       it 'is successful' do
@@ -38,17 +38,18 @@
     end
 
     context 'not using database auth' do
-      before do
-        @cached_database_auth = ENV['DATABASE_AUTH']
-        @cached_sso_logout_url = ENV['SSO_LOGOUT_URL']
+      around do |example|
+        cached_database_auth = ENV['DATABASE_AUTH']
+        cached_sso_logout_url = ENV['SSO_LOGOUT_URL']
         ENV['DATABASE_AUTH'] = 'false'
         ENV['SSO_LOGOUT_URL'] = 'https://shibboleth.example.com/idp/logout.jsp'
-        allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new("production"))
+        example.run
+        ENV['DATABASE_AUTH'] = cached_database_auth
+        ENV['SSO_LOGOUT_URL'] = cached_sso_logout_url
       end
 
-      after do
-        ENV['DATABASE_AUTH'] = @cached_database_auth
-        ENV['SSO_LOGOUT_URL'] = @cached_sso_logout_url
+      before do
+        allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new("production"))
       end
 
       it 'redirects to shibboleth logout url' do
@@ -57,4 +58,4 @@
       end
     end
   end
-end
+end

spec/features/boxc_redirect_spec.rb

@@ -2,23 +2,28 @@
 
 # before actions
 RSpec.feature 'boxc redirects' do
-  cached_redirect_file_path = ENV['REDIRECT_FILE_PATH']
-  cached_redirect_old_domain = ENV['REDIRECT_OLD_DOMAIN']
-  cached_redirect_new_domain = ENV['REDIRECT_NEW_DOMAIN']
-  tempfile = Tempfile.new('redirect_uuids.csv', 'spec/fixtures/')
-
-  before(:all) do
-    ENV['REDIRECT_FILE_PATH'] = 'spec/fixtures/redirect_uuids.csv'
+  let(:tempfile) { Tempfile.new('redirect_uuids.csv', 'spec/fixtures/') }
+  before do
+    tempfile
     File.open(ENV['REDIRECT_FILE_PATH'], 'w') do |f|
       f.puts 'uuid,new_path'
     end
-    ENV['REDIRECT_OLD_DOMAIN'] = ENV['HYRAX_HOST'].gsub('https://','')
-    ENV['REDIRECT_NEW_DOMAIN'] = 'dcr-test.lib.unc.edu'
     stub_request(:any, 'https://dcr-test.lib.unc.edu/list/uuid:02fc897a-12b6-4b81-91e4-b5e29cb683a6').to_return(status: 200)
   end
 
-  after(:all) do
+  after do
     tempfile.unlink
+    File.delete('spec/fixtures/redirect_uuids.csv') if File.exist?('spec/fixtures/redirect_uuids.csv')
+  end
+  around(:all) do |example|
+    cached_redirect_file_path = ENV['REDIRECT_FILE_PATH']
+    cached_redirect_old_domain = ENV['REDIRECT_OLD_DOMAIN']
+    cached_redirect_new_domain = ENV['REDIRECT_NEW_DOMAIN']
+    ENV['REDIRECT_FILE_PATH'] = 'spec/fixtures/redirect_uuids.csv'
+    ENV['REDIRECT_OLD_DOMAIN'] = ENV['HYRAX_HOST'].gsub('https://','')
+    ENV['REDIRECT_NEW_DOMAIN'] = 'dcr-test.lib.unc.edu'
+    # example.run is where the test actually runs
+    example.run
     ENV['REDIRECT_FILE_PATH'] = cached_redirect_file_path
     ENV['REDIRECT_OLD_DOMAIN'] = cached_redirect_old_domain
     ENV['REDIRECT_NEW_DOMAIN'] = cached_redirect_new_domain

spec/helpers/hyc_helper_spec.rb

@@ -38,12 +38,10 @@
   end
 
   describe '#redirect_lookup' do
-    cached_redirect_file_path = ENV['REDIRECT_FILE_PATH']
-    tempfile = Tempfile.new('redirect_uuids.csv', 'spec/fixtures/')
+    let(:tempfile) { Tempfile.new('redirect_uuids.csv', 'spec/fixtures/') }
     let(:article) { Article.create(title: ['new article'], visibility: 'open') }
 
     before do
-      ENV['REDIRECT_FILE_PATH'] = 'spec/fixtures/redirect_uuids.csv'
       File.open(ENV['REDIRECT_FILE_PATH'], 'w') do |f|
         f.puts 'uuid,new_path'
         f.puts "02fc897a-12b6-4b81-91e4-b5e29cb683a6,articles/#{article.id}"
@@ -52,6 +50,13 @@
 
     after do
       tempfile.unlink
+      File.delete('spec/fixtures/redirect_uuids.csv') if File.exist?('spec/fixtures/redirect_uuids.csv')
+    end
+
+    around do |example|
+      cached_redirect_file_path = ENV['REDIRECT_FILE_PATH']
+      ENV['REDIRECT_FILE_PATH'] = 'spec/fixtures/redirect_uuids.csv'
+      example.run
       ENV['REDIRECT_FILE_PATH'] = cached_redirect_file_path
     end
 

spec/jobs/deregister_longleaf_job_spec.rb

@@ -5,15 +5,15 @@
 require 'fileutils'
 
 RSpec.describe DeregisterLongleafJob, type: :job do
-  
+
   let(:admin_user) do
     User.find_by_user_key('admin')
   end
-  
+
   let(:binary_dir) { File.join(Rails.root, "tmp/fcrepo4-test-data/fcrepo.binary.directory/") }
-  
+
   let(:ll_home_dir) { Dir.mktmpdir('ll_home') }
-  
+
   let(:repository_file) do
     Hydra::PCDM::File.new do |f|
       tmp_file = Tempfile.new
@@ -24,49 +24,53 @@
       f.mime_type = 'application/pdf'
     end
   end
-  
+
   let(:file_set) { FileSet.new }
-  
+
   let(:job) { DeregisterLongleafJob.new }
-  
+
   after do
     FileUtils.rm_rf([ll_home_dir, binary_dir])
   end
-  
+
   context 'With minimal config' do
     let(:output_path) { File.join(ll_home_dir, "output.txt")}
-    
+
     let(:longleaf_script) do
       path = File.join(ll_home_dir, "llcommand.sh")
       File.write(path, "#!/usr/bin/env bash\n" +
           "echo $@ > #{output_path.to_s}")
       path
     end
-    
+
     before do
+      allow(Hydra::Works::VirusCheckerService).to receive(:file_has_virus?) { false }
       FileUtils.chmod("u+x", longleaf_script)
-      ENV["LONGLEAF_STORAGE_PATH"] = binary_dir
-      ENV["LONGLEAF_BASE_COMMAND"] = longleaf_script
-
+
       file_set.apply_depositor_metadata admin_user.user_key
       file_set.save!
       file_set.original_file = repository_file
       file_set.save!
     end
-
-    after do
-      ENV.delete("LONGLEAF_STORAGE_PATH")
-      ENV.delete("LONGLEAF_BASE_COMMAND")
+
+    around do |example|
+      cached_storage_path = ENV["LONGLEAF_STORAGE_PATH"]
+      cached_base_command = ENV["LONGLEAF_BASE_COMMAND"]
+      ENV["LONGLEAF_STORAGE_PATH"] = binary_dir
+      ENV["LONGLEAF_BASE_COMMAND"] = longleaf_script
+      example.run
+      ENV["LONGLEAF_STORAGE_PATH"] = cached_storage_path
+      ENV["LONGLEAF_BASE_COMMAND"] = cached_base_command
     end
-    
+
     it 'calls deregistration script with the expected parameters' do
       job.perform(file_set)
-      
+
       arguments = File.read(output_path)
-      
+
       binary_path = File.join(binary_dir, "12/e5/f2/12e5f2da18960dc085ca27bec1ae9e3245389cb1")
-      
+
       expect(arguments).to match(Regexp.new(".*deregister -f #{binary_path}"))
     end
   end
-end
+end

spec/services/admin_set_select_service_spec.rb

@@ -54,9 +54,13 @@
     end
 
     context "when no matches found" do
-      before do
-        ENV['DEFAULT_ADMIN_SET']='default'
+      around do |example|
+        cached_default_admin_set = ENV['DEFAULT_ADMIN_SET']
+        ENV['DEFAULT_ADMIN_SET'] = 'default'
+        example.run
+        ENV['DEFAULT_ADMIN_SET'] = cached_default_admin_set
       end
+
       it "returns the default admin set" do
         expect(service.select("HonorsThesis", nil, [['default', admin_set.id], ['mediated', 'mediated-id']]))
             .to eq admin_set.id