Skip to content

Commit

Permalink
Get CI consistently green (#7)
Browse files Browse the repository at this point in the history
The goal is to get CI green so that we can make behavior and structural
changes with confidence.

I've found the following sources of test pollution:
- ENV values being written to and not cleaned up between tests
- `Annotate` has the instance variable, `@has_set_defaults`, which can
persist between tests
  • Loading branch information
drwl authored Feb 17, 2023
1 parent 00c56dd commit 9f2e770
Show file tree
Hide file tree
Showing 14 changed files with 130 additions and 211 deletions.
1 change: 0 additions & 1 deletion lib/annotate_rb/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ def initialize
HELP_MAPPING = %w(-h -? --help).to_set
VERSION_MAPPING = %w(-v --version).to_set


def run(args = ARGV)
_original_argv = ARGV.dup

Expand Down
15 changes: 0 additions & 15 deletions spec/lib/annotate/annotate_models_annotate_model_file_spec.rb
Original file line number Diff line number Diff line change
@@ -1,26 +1,11 @@
# encoding: utf-8
require_relative '../../spec_helper'
require 'annotate/annotate_models'
require 'annotate/active_record_patch'
require 'active_support/core_ext/string'
require 'files'
require 'tmpdir'

RSpec.describe AnnotateModels do
MAGIC_COMMENTS = [
'# encoding: UTF-8',
'# coding: UTF-8',
'# -*- coding: UTF-8 -*-',
'#encoding: utf-8',
'# encoding: utf-8',
'# -*- encoding : utf-8 -*-',
"# encoding: utf-8\n# frozen_string_literal: true",
"# frozen_string_literal: true\n# encoding: utf-8",
'# frozen_string_literal: true',
'#frozen_string_literal: false',
'# -*- frozen_string_literal : true -*-'
].freeze

describe '.annotate_model_file' do
before do
class Foo < ActiveRecord::Base; end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# encoding: utf-8
require_relative '../../spec_helper'
require 'annotate/annotate_models'
require 'annotate/active_record_patch'
require 'active_support/core_ext/string'
Expand All @@ -9,20 +8,6 @@
RSpec.describe AnnotateModels do
include AnnotateTestHelpers

MAGIC_COMMENTS = [
'# encoding: UTF-8',
'# coding: UTF-8',
'# -*- coding: UTF-8 -*-',
'#encoding: utf-8',
'# encoding: utf-8',
'# -*- encoding : utf-8 -*-',
"# encoding: utf-8\n# frozen_string_literal: true",
"# frozen_string_literal: true\n# encoding: utf-8",
'# frozen_string_literal: true',
'#frozen_string_literal: false',
'# -*- frozen_string_literal : true -*-'
].freeze

describe 'annotating a file' do
before do
@model_dir = Dir.mktmpdir('annotate_models')
Expand All @@ -41,28 +26,8 @@ class User < ActiveRecord::Base
Annotate::Helpers.reset_options(Annotate::Constants::ALL_ANNOTATE_OPTIONS)
end

def write_model(file_name, file_content)
fname = File.join(@model_dir, file_name)
FileUtils.mkdir_p(File.dirname(fname))
File.open(fname, 'wb') { |f| f.write file_content }

[fname, file_content]
end

def annotate_one_file(options = {})
Annotate.set_defaults(options)
options = Annotate.setup_options(options)
AnnotateModels.annotate_one_file(@model_file_name, @schema_info, :position_in_class, options)

# Wipe settings so the next call will pick up new values...
Annotate.instance_variable_set('@has_set_defaults', false)
Annotate::Constants::POSITION_OPTIONS.each { |key| ENV[key.to_s] = '' }
Annotate::Constants::FLAG_OPTIONS.each { |key| ENV[key.to_s] = '' }
Annotate::Constants::PATH_OPTIONS.each { |key| ENV[key.to_s] = '' }
end

# TODO: Check out why this test fails due to test pollution
xdescribe 'frozen option' do
describe 'frozen option' do
it "should abort without existing annotation when frozen: true " do
expect { annotate_one_file frozen: true }.to raise_error SystemExit, /user.rb needs to be updated, but annotate was run with `--frozen`./
end
Expand Down
104 changes: 67 additions & 37 deletions spec/lib/annotate/annotate_models_annotating_a_file_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# encoding: utf-8
require_relative '../../spec_helper'
require 'annotate/annotate_models'
require 'annotate/active_record_patch'
require 'active_support/core_ext/string'
Expand All @@ -8,20 +7,7 @@

RSpec.describe AnnotateModels do
include AnnotateTestHelpers

MAGIC_COMMENTS = [
'# encoding: UTF-8',
'# coding: UTF-8',
'# -*- coding: UTF-8 -*-',
'#encoding: utf-8',
'# encoding: utf-8',
'# -*- encoding : utf-8 -*-',
"# encoding: utf-8\n# frozen_string_literal: true",
"# frozen_string_literal: true\n# encoding: utf-8",
'# frozen_string_literal: true',
'#frozen_string_literal: false',
'# -*- frozen_string_literal : true -*-'
].freeze
include AnnotateTestConstants

describe 'annotating a file' do
before do
Expand All @@ -41,36 +27,80 @@ class User < ActiveRecord::Base
Annotate::Helpers.reset_options(Annotate::Constants::ALL_ANNOTATE_OPTIONS)
end

def write_model(file_name, file_content)
fname = File.join(@model_dir, file_name)
FileUtils.mkdir_p(File.dirname(fname))
File.open(fname, 'wb') { |f| f.write file_content }
context "with 'before'" do
let(:position) { 'before' }

it "should put annotation before class if :position == 'before'" do
annotate_one_file position: position
expect(File.read(@model_file_name))
.to eq("#{@schema_info}#{@file_content}")
end
end

context "with :before" do
let(:position) { :before }

[fname, file_content]
it "should put annotation before class if :position == :before" do
annotate_one_file position: position
expect(File.read(@model_file_name))
.to eq("#{@schema_info}#{@file_content}")
end
end

def annotate_one_file(options = {})
Annotate.set_defaults(options)
options = Annotate.setup_options(options)
AnnotateModels.annotate_one_file(@model_file_name, @schema_info, :position_in_class, options)
context "with 'top'" do
let(:position) { 'top' }

# Wipe settings so the next call will pick up new values...
Annotate.instance_variable_set('@has_set_defaults', false)
Annotate::Constants::POSITION_OPTIONS.each { |key| ENV[key.to_s] = '' }
Annotate::Constants::FLAG_OPTIONS.each { |key| ENV[key.to_s] = '' }
Annotate::Constants::PATH_OPTIONS.each { |key| ENV[key.to_s] = '' }
it "should put annotation before class if :position == 'top'" do
annotate_one_file position: position
expect(File.read(@model_file_name))
.to eq("#{@schema_info}#{@file_content}")
end
end

['before', :before, 'top', :top].each do |position|
it "should put annotation before class if :position == #{position}" do
context "with :top" do
let(:position) { :top }

it "should put annotation before class if :position == :top" do
annotate_one_file position: position
expect(File.read(@model_file_name))
.to eq("#{@schema_info}#{@file_content}")
end
end

['after', :after, 'bottom', :bottom].each do |position|
it "should put annotation after class if position: #{position}" do
context "with 'after'" do
let(:position) { 'after' }

it "should put annotation after class if position: 'after'" do
annotate_one_file position: position
expect(File.read(@model_file_name))
.to eq("#{@file_content}\n#{@schema_info}")
end
end

context "with :after" do
let(:position) { :after }

it "should put annotation after class if position: :after" do
annotate_one_file position: position
expect(File.read(@model_file_name))
.to eq("#{@file_content}\n#{@schema_info}")
end
end

context "with 'bottom'" do
let(:position) { 'bottom' }

it "should put annotation after class if position: 'bottom'" do
annotate_one_file position: position
expect(File.read(@model_file_name))
.to eq("#{@file_content}\n#{@schema_info}")
end
end

context "with :bottom" do
let(:position) { :bottom }

it "should put annotation after class if position: :bottom" do
annotate_one_file position: position
expect(File.read(@model_file_name))
.to eq("#{@file_content}\n#{@schema_info}")
Expand Down Expand Up @@ -196,7 +226,7 @@ class Foo::User < ActiveRecord::Base
end

it 'should not touch magic comments' do
MAGIC_COMMENTS.each do |magic_comment|
AnnotateTestConstants::MAGIC_COMMENTS.each do |magic_comment|
write_model 'user.rb', <<~EOS
#{magic_comment}
class User < ActiveRecord::Base
Expand All @@ -216,7 +246,7 @@ class User < ActiveRecord::Base

it 'adds an empty line between magic comments and annotation (position :before)' do
content = "class User < ActiveRecord::Base\nend\n"
MAGIC_COMMENTS.each do |magic_comment|
AnnotateTestConstants::MAGIC_COMMENTS.each do |magic_comment|
model_file_name, = write_model 'user.rb', "#{magic_comment}\n#{content}"

annotate_one_file position: :before
Expand All @@ -228,7 +258,7 @@ class User < ActiveRecord::Base

it 'only keeps a single empty line around the annotation (position :before)' do
content = "class User < ActiveRecord::Base\nend\n"
MAGIC_COMMENTS.each do |magic_comment|
AnnotateTestConstants::MAGIC_COMMENTS.each do |magic_comment|
schema_info = AnnotateModels::SchemaInfo.generate(@klass, '== Schema Info')
model_file_name, = write_model 'user.rb', "#{magic_comment}\n\n\n\n#{content}"

Expand All @@ -240,7 +270,7 @@ class User < ActiveRecord::Base

it 'does not change whitespace between magic comments and model file content (position :after)' do
content = "class User < ActiveRecord::Base\nend\n"
MAGIC_COMMENTS.each do |magic_comment|
AnnotateTestConstants::MAGIC_COMMENTS.each do |magic_comment|
model_file_name, = write_model 'user.rb', "#{magic_comment}\n#{content}"

annotate_one_file position: :after
Expand Down
30 changes: 6 additions & 24 deletions spec/lib/annotate/annotate_models_get_model_class_spec.rb
Original file line number Diff line number Diff line change
@@ -1,31 +1,11 @@
# encoding: utf-8
require_relative '../../spec_helper'
require 'annotate/annotate_models'
require 'annotate/active_record_patch'
require 'active_support/core_ext/string'
require 'tmpdir'

RSpec.describe AnnotateModels do
MAGIC_COMMENTS = [
'# encoding: UTF-8',
'# coding: UTF-8',
'# -*- coding: UTF-8 -*-',
'#encoding: utf-8',
'# encoding: utf-8',
'# -*- encoding : utf-8 -*-',
"# encoding: utf-8\n# frozen_string_literal: true",
"# frozen_string_literal: true\n# encoding: utf-8",
'# frozen_string_literal: true',
'#frozen_string_literal: false',
'# -*- frozen_string_literal : true -*-'
].freeze

describe '.get_model_class' do
before do
AnnotateModels.model_dir = Dir.mktmpdir('annotate_models')
end

# TODO: use 'files' gem instead
def create(filename, file_content)
File.join(AnnotateModels.model_dir[0], filename).tap do |path|
FileUtils.mkdir_p(File.dirname(path))
Expand All @@ -35,7 +15,8 @@ def create(filename, file_content)
end
end

before :each do
before do
AnnotateModels.model_dir = Dir.mktmpdir('annotate_models')
create(filename, file_content)
end

Expand Down Expand Up @@ -254,7 +235,7 @@ class LoadedClass < ActiveRecord::Base
EOS
end

before :each do
before do
path = File.expand_path(filename, AnnotateModels.model_dir[0])
Kernel.load(path)
expect(Kernel).not_to receive(:require)
Expand All @@ -269,7 +250,7 @@ class LoadedClass < ActiveRecord::Base
dir = Array.new(8) { (0..9).to_a.sample(random: Random.new) }.join

context "when class SubdirLoadedClass is defined in \"#{dir}/subdir_loaded_class.rb\"" do
before :each do
before do
$LOAD_PATH.unshift(File.join(AnnotateModels.model_dir[0], dir))

path = File.expand_path(filename, AnnotateModels.model_dir[0])
Expand Down Expand Up @@ -297,7 +278,7 @@ class SubdirLoadedClass < ActiveRecord::Base
end

context 'when two class exist' do
before :each do
before do
create(filename_2, file_content_2)
end

Expand All @@ -319,6 +300,7 @@ class Foo < ActiveRecord::Base

let :file_content_2 do
<<-EOS
module Bar; end
class Bar::Foo
end
EOS
Expand Down
15 changes: 0 additions & 15 deletions spec/lib/annotate/annotate_models_get_model_files_spec.rb
Original file line number Diff line number Diff line change
@@ -1,26 +1,11 @@
# encoding: utf-8
require_relative '../../spec_helper'
require 'annotate/annotate_models'
require 'annotate/active_record_patch'
require 'active_support/core_ext/string'
require 'files'
require 'tmpdir'

RSpec.describe AnnotateModels do
MAGIC_COMMENTS = [
'# encoding: UTF-8',
'# coding: UTF-8',
'# -*- coding: UTF-8 -*-',
'#encoding: utf-8',
'# encoding: utf-8',
'# -*- encoding : utf-8 -*-',
"# encoding: utf-8\n# frozen_string_literal: true",
"# frozen_string_literal: true\n# encoding: utf-8",
'# frozen_string_literal: true',
'#frozen_string_literal: false',
'# -*- frozen_string_literal : true -*-'
].freeze

describe '.get_model_files' do
subject { described_class.get_model_files(options) }

Expand Down
15 changes: 0 additions & 15 deletions spec/lib/annotate/annotate_models_get_patterns_spec.rb
Original file line number Diff line number Diff line change
@@ -1,26 +1,11 @@
# encoding: utf-8
require_relative '../../spec_helper'
require 'annotate/annotate_models'
require 'annotate/active_record_patch'
require 'active_support/core_ext/string'
require 'files'
require 'tmpdir'

RSpec.describe AnnotateModels do
MAGIC_COMMENTS = [
'# encoding: UTF-8',
'# coding: UTF-8',
'# -*- coding: UTF-8 -*-',
'#encoding: utf-8',
'# encoding: utf-8',
'# -*- encoding : utf-8 -*-',
"# encoding: utf-8\n# frozen_string_literal: true",
"# frozen_string_literal: true\n# encoding: utf-8",
'# frozen_string_literal: true',
'#frozen_string_literal: false',
'# -*- frozen_string_literal : true -*-'
].freeze

describe '.get_patterns' do
subject { AnnotateModels.get_patterns(options, pattern_type) }

Expand Down
Loading

0 comments on commit 9f2e770

Please sign in to comment.