From 02e0459998fd8249f583d47f37665ba4dfd8c628 Mon Sep 17 00:00:00 2001 From: Bue Petersen Date: Mon, 1 May 2017 20:53:19 +0200 Subject: [PATCH] #98 Changes, examples and drafts from test work shop (#99) We together a series of commits for moving to rspec testing, updating some code to be easier to test and added new tests as examples. Commits: * Added example test for #98. Run with rspec --format=doc * #98 unit test and refactor example * Added command line test for #98 * #98 More rspec test examples * Updated gem files for test. See #98 * Added other test gem dependencies --- .gitignore | 2 +- .rspec | 2 + Dockerfile | 2 +- Gemfile | 11 ++- Gemfile.lock | 15 ++++ Rakefile | 5 ++ lib/core.rb | 32 +++++++ lib/model.rb | 107 +++++++++++------------ pac.rb | 29 +----- spec/command_line_interpretation_spec.rb | 38 ++++++++ spec/pactask_spec.rb | 38 ++++++++ spec/spec_helper.rb | 103 ++++++++++++++++++++++ spec/task_spec.rb | 60 +++++++++++++ test/unit/modeltest.rb | 50 ++++++++--- 14 files changed, 394 insertions(+), 100 deletions(-) create mode 100644 .rspec create mode 100644 spec/command_line_interpretation_spec.rb create mode 100644 spec/pactask_spec.rb create mode 100644 spec/spec_helper.rb create mode 100644 spec/task_spec.rb diff --git a/.gitignore b/.gitignore index 357ec73..bd14d59 100644 --- a/.gitignore +++ b/.gitignore @@ -12,4 +12,4 @@ default.md default_ids.md default.pdf default-generated.* - +.bundle diff --git a/.rspec b/.rspec new file mode 100644 index 0000000..83e16f8 --- /dev/null +++ b/.rspec @@ -0,0 +1,2 @@ +--color +--require spec_helper diff --git a/Dockerfile b/Dockerfile index 78adff3..ac9b3b8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -40,7 +40,7 @@ COPY Gemfile.lock /usr/src/app/ #Ruby knows best how to install this particular version of PAC #This means that this dockerfile can build any version of PAC. -RUN bundle install +RUN bundle install --without=test_gems COPY . /usr/src/app diff --git a/Gemfile b/Gemfile index db686ee..0f1ba55 100644 --- a/Gemfile +++ b/Gemfile @@ -8,9 +8,14 @@ gem 'trac4r', :require => false gem 'docopt' gem 'ruby-fogbugz', :require => false gem 'flexmock' -gem 'simplecov', :require => false -gem 'simplecov-rcov', :require => false -gem 'ci_reporter_test_unit' gem 'zip' gem 'liquid' gem 'xml-simple', '~> 1.1', '>= 1.1.5' + +group :test_gems do + gem 'rspec' + gem 'simplecov' + gem 'simplecov-rcov' + gem 'ci_reporter_test_unit' +end + diff --git a/Gemfile.lock b/Gemfile.lock index 968da40..6c6fba3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -12,6 +12,7 @@ GEM safe_yaml (~> 1.0.0) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) + diff-lcs (1.3) docile (1.1.5) docopt (0.5.0) faraday (0.9.2) @@ -69,6 +70,19 @@ GEM rake (11.1.2) rdoc (4.2.2) json (~> 1.4) + rspec (3.5.0) + rspec-core (~> 3.5.0) + rspec-expectations (~> 3.5.0) + rspec-mocks (~> 3.5.0) + rspec-core (3.5.4) + rspec-support (~> 3.5.0) + rspec-expectations (3.5.0) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.5.0) + rspec-mocks (3.5.0) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.5.0) + rspec-support (3.5.0) ruby-fogbugz (0.2.1) crack (~> 0.4) rugged (0.24.0) @@ -110,6 +124,7 @@ DEPENDENCIES mercurial-ruby pdfkit rake + rspec ruby-fogbugz rugged (~> 0.24.0) simplecov diff --git a/Rakefile b/Rakefile index f84b79e..e1bb7da 100644 --- a/Rakefile +++ b/Rakefile @@ -31,6 +31,11 @@ task :functional_vcs do end end +#Example +task :rspec do + `rspec --format='html' --out='results.html'` +end + task :coverage do ENV['COVERAGE'] = 'on' Rake::Task['test'].execute diff --git a/lib/core.rb b/lib/core.rb index e691ee5..a8770fc 100644 --- a/lib/core.rb +++ b/lib/core.rb @@ -7,6 +7,38 @@ module Core extend self + def cli_text(file) + cli = < [--to=] [options] [-v...] [-q...] [-c ( )]... + #{file} from-latest-tag [--to=] [options] [-v...] [-q...] [-c ]... + #{file} -h|--help + + Options: + -h --help Show this screen. + + --from Specify where to stop searching for commit. For git this takes anything that rev-parse accepts. Such as HEAD~3 / Git sha or tag name. + + --from-latest-tag Looks for the newest commit that the tag with points to. + + --settings= Path to the settings file used. If nothing is specified default_settings.yml is used + + --properties= + + Allows you to pass in additional variables to the Liquid templates. Must be in JSON format. Namespaced under properties.* in + your Liquid templates. Referenced like so '{{properties.[your-variable]}}' in your templates. + + JSON keys and values should be wrapped in quotation marks '"' like so: --properties='{ "title":"PAC Changelog" }' + + -v More verbose output. Can be repeated to increase output verbosity or to cancel out -q + + -q Less verbose output. Can be repeated for more silence or to cancel out -v + + -c Override username and password. Example: `-c my_user my_password jira`. This will set username and password for task system jira. +DOCOPT + cli + end + def settings if defined?(@@settings).nil? {} diff --git a/lib/model.rb b/lib/model.rb index 628e2ca..95fab79 100644 --- a/lib/model.rb +++ b/lib/model.rb @@ -4,26 +4,34 @@ module Model class PACTask def initialize(task_id = nil) #Lookup key for task management system - @task_id = task_id + #We assume that task_id is a string + raise ArgumentError.new("PACTask init error, task id is not a string.") if not (task_id == nil or task_id.respond_to?(:to_str)) + @task_id = task_id #Commits tied to this task - @commit_collection = PACCommitCollection.new + @commit_collection = PACCommitCollection.new #Data from task management systems(s) @attributes = { } #Key that determines which system(s) we need to look in for data - @applies_to = Set.new + @applies_to = Set.new #Assigned label. Used in templates so that you can group your tasks using labels. @label = Set.new - end - def applies_to - @applies_to - end - - def data - @data + # Initialized with nil - will be populated by decorators + # FIXME is it default nil ? + @data = nil end + attr_accessor :commit_collection + attr_accessor :task_id + attr_accessor :data + attr_reader :label + attr_reader :applies_to + attr_reader :attributes + attr_accessor :commits + + # accepts string def applies_to=(val) + raiseArgumentError.new("apllies_to method accepts only a string") if not val.is_a? String @applies_to << val end @@ -31,16 +39,12 @@ def add_commit(commit) @commit_collection.add(commit) end - def commits - @commit_collection.commits + def attributes=(value) + @attributes = value end def to_s - @task_id - end - - def label - @label + @task_id end #Apppend label to the set of existing labels. This prevents duplicate labels. @@ -54,31 +58,20 @@ def clear_labels #We need to override the equals method. A task is uniquely identified by it's identifier (id). This is usually what #is used to fetch additonal information from the task management system - def ==(other) + def ==(other) @task_id == other.task_id end def to_liquid - hash = { - 'task_id' => @task_id, - 'commits' => @commit_collection, + hash = { + 'task_id' => @task_id, + 'commits' => @commit_collection, 'attributes' => attributes, 'label' => label, 'data' => data } hash end - - def attributes - @attributes - end - - def attributes=(value) - @attributes = value - end - - attr_accessor :commit_collection - attr_accessor :task_id end #Model object representing a collection of tasks. Includes logic to append commits to existing tasks. @@ -88,22 +81,22 @@ def initialize @tasks = [] end - #When you add a task to a collection. It will automatically add new commits to an existing task. You will + #When you add a task to a collection. It will automatically add new commits to an existing task. You will #not get duplicate tasks in the collection. def add(*t) - t.flatten.each do |task| + t.flatten.each do |task| unless @tasks.include? task @tasks << task else task.commit_collection.each do |c| - @tasks[@tasks.index(task)].commit_collection.add(c) - end + @tasks[@tasks.index(task)].commit_collection.add(c) + end end end end #Enumeartion method. So that PACTaskCollection.each yields a PACTask object - def each + def each @tasks.each { |task| yield task } end @@ -113,7 +106,7 @@ def length def unreferenced_commits output = @tasks.select { |t| t.task_id.nil? } - if output.first.nil? + if output.first.nil? [] else output.first.commits @@ -125,14 +118,14 @@ def referenced_tasks end #Indexer method for each collected task - def [](task_id) + def [](task_id) @tasks.select {|t| t.task_id == task_id}.first end #Group each task discovered by it's label def by_label labelled = Hash.new - @tasks.each { |t| + @tasks.each { |t| t.label.each { |t_label| unless labelled.include? t_label labelled[t_label] = [] @@ -145,7 +138,7 @@ def by_label labelled end - def to_liquid + def to_liquid liquid_hash = { 'tasks' => @tasks, 'referenced' => referenced_tasks, 'unreferenced' => unreferenced_commits } liquid_hash.merge! by_label liquid_hash @@ -155,7 +148,7 @@ def to_liquid end class PACCommitCollection - def initialize + def initialize @commits = [] end @@ -180,15 +173,15 @@ def count_without @commits.select{ |c| c.referenced == false }.size end - def health + def health count_with.to_f / count end - def to_liquid + def to_liquid @commits end - attr_accessor :commits + attr_accessor :commits end class PACCommit @@ -196,25 +189,25 @@ def initialize(sha, message = nil, date = nil) @sha = sha @message = message @referenced = false - @date = date + @date = date end - def to_liquid - { + def to_liquid + { 'sha' => @sha, - 'message' => @message, - 'header' => header, + 'message' => @message, + 'header' => header, 'referenced' => referenced, 'shortsha' => shortsha } end - def ==(other) - @sha == other.sha + def ==(other) + @sha == other.sha end - def header - @message.split(/\n/).first + def header + @message.split(/\n/).first end #Get default abbreviation from Git @@ -226,7 +219,7 @@ def shortsha(n = 7) #returned for unmatch commits. That is a PACTask with an id of 'nil'. def matchtask(regex, split = nil) tasks = [] - regex.each do |r| + regex.each do |r| @message.scan(eval(r['pattern'])).each do |arr| if split.nil? task = PACTask.new(arr[0]) @@ -240,12 +233,12 @@ def matchtask(regex, split = nil) task.add_commit(self) task.label = r['label'] self.referenced = true - tasks << task + tasks << task end end end end - + tasks end diff --git a/pac.rb b/pac.rb index 0c57442..333889b 100755 --- a/pac.rb +++ b/pac.rb @@ -5,34 +5,7 @@ require_relative 'lib/core' require_relative 'lib/report' -doc = < [--to=] [options] [-v...] [-q...] [-c ( )]... - #{__FILE__} from-latest-tag [--to=] [options] [-v...] [-q...] [-c ]... - #{__FILE__} -h|--help - -Options: - -h --help Show this screen. - - --from Specify where to stop searching for commit. For git this takes anything that rev-parse accepts. Such as HEAD~3 / Git sha or tag name. - - --from-latest-tag Looks for the newest commit that the tag with points to. - - --settings= Path to the settings file used. If nothing is specified default_settings.yml is used - - --properties= - - Allows you to pass in additional variables to the Liquid templates. Must be in JSON format. Namespaced under properties.* in - your Liquid templates. Referenced like so '{{properties.[your-variable]}}' in your templates. - - JSON keys and values should be wrapped in quotation marks '"' like so: --properties='{ "title":"PAC Changelog" }' - - -v More verbose output. Can be repeated to increase output verbosity or to cancel out -q - - -q Less verbose output. Can be repeated for more silence or to cancel out -v - - -c Override username and password. Example: `-c my_user my_password jira`. This will set username and password for task system jira. -DOCOPT +doc = Core.cli_text(__FILE__) begin require "pp" diff --git a/spec/command_line_interpretation_spec.rb b/spec/command_line_interpretation_spec.rb new file mode 100644 index 0000000..4ea8430 --- /dev/null +++ b/spec/command_line_interpretation_spec.rb @@ -0,0 +1,38 @@ +require_relative '../lib/core' +require 'docopt' + +RSpec.describe "Command line interactions" do + describe "Commands that must be checked" do + + context "When the user wants to get help with the --help option" do + it "prints out the help message on the screen" do + ARGV = "--help" + expect { Docopt::docopt(Core.cli_text(__FILE__)) }.to raise_error(Docopt::Exit) + end + end + + context "When an illegalt parameter is passed" do + it "prints the help message to the screen and informs the user" do + ARGV = "--mads" + #TODO: Check error message + expect { Docopt::docopt(Core.cli_text(__FILE__)) }.to raise_error(Docopt::Exit) + end + end + + context "When from-latest-tag is used" do + it "does as we expect" do + ARGV = "from-latest-tag '*' --to='somesha' --settings='pac_settings.yml'" + #TODO: We should add some kind of validation test here + input = Docopt::docopt(Core.cli_text(__FILE__)) + end + end + + context "When the command from is used" do + it "does as we expect" do + ARGV = "from 'sha' --to='somesha' --settings='pac_settings.yml' -c 'user' 'pass' 'tasksystem'" + input = Docopt::docopt(Core.cli_text(__FILE__)) + puts input + end + end + end +end \ No newline at end of file diff --git a/spec/pactask_spec.rb b/spec/pactask_spec.rb new file mode 100644 index 0000000..1e624df --- /dev/null +++ b/spec/pactask_spec.rb @@ -0,0 +1,38 @@ +require_relative '../lib/model.rb' + +RSpec.describe Model do + describe "class PACTask" do + + describe ".new" do + context "Task initialized" do + let(:pt) { Model::PACTask.new() } + it "without task id should have task_id nil" do + expect(pt.task_id).to eq(nil) + end + + let(:pt_id) { Model::PACTask.new(3) } + it "with something else than a string should raiseArgument error as we expect it to be a string later" do + expect{Model::PACTask.new(3)}.to raise_error(ArgumentError) + end + end + end + + + + describe "applies_to" do + let(:pt) { Model::PACTask.new() } + + context "Given a new task" do + it "applies_to should be empty" do + expect(pt.applies_to.length).to be 0 + end + + it "applies_to should return the set of applied tasks systems" do + pt.applies_to = 'mytaskssystem' + ts = Set.new ['mytaskssystem'] + expect(pt.applies_to).to eq(ts) # not object identity, just content + end + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb new file mode 100644 index 0000000..47b39ce --- /dev/null +++ b/spec/spec_helper.rb @@ -0,0 +1,103 @@ +# This file was generated by the `rspec --init` command. Conventionally, all +# specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`. +# The generated `.rspec` file contains `--require spec_helper` which will cause +# this file to always be loaded, without a need to explicitly require it in any +# files. +# +# Given that it is always loaded, you are encouraged to keep this file as +# light-weight as possible. Requiring heavyweight dependencies from this file +# will add to the boot time of your test suite on EVERY test run, even for an +# individual file that may not need all of that loaded. Instead, consider making +# a separate helper file that requires the additional dependencies and performs +# the additional setup, and require it from the spec files that actually need +# it. +# +# The `.rspec` file also contains a few flags that are not defaults but that +# users commonly want. +# +# See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration +RSpec.configure do |config| + # rspec-expectations config goes here. You can use an alternate + # assertion/expectation library such as wrong or the stdlib/minitest + # assertions if you prefer. + config.expect_with :rspec do |expectations| + # This option will default to `true` in RSpec 4. It makes the `description` + # and `failure_message` of custom matchers include text for helper methods + # defined using `chain`, e.g.: + # be_bigger_than(2).and_smaller_than(4).description + # # => "be bigger than 2 and smaller than 4" + # ...rather than: + # # => "be bigger than 2" + expectations.include_chain_clauses_in_custom_matcher_descriptions = true + end + + # rspec-mocks config goes here. You can use an alternate test double + # library (such as bogus or mocha) by changing the `mock_with` option here. + config.mock_with :rspec do |mocks| + # Prevents you from mocking or stubbing a method that does not exist on + # a real object. This is generally recommended, and will default to + # `true` in RSpec 4. + mocks.verify_partial_doubles = true + end + + # This option will default to `:apply_to_host_groups` in RSpec 4 (and will + # have no way to turn it off -- the option exists only for backwards + # compatibility in RSpec 3). It causes shared context metadata to be + # inherited by the metadata hash of host groups and examples, rather than + # triggering implicit auto-inclusion in groups with matching metadata. + config.shared_context_metadata_behavior = :apply_to_host_groups + +# The settings below are suggested to provide a good initial experience +# with RSpec, but feel free to customize to your heart's content. +=begin + # This allows you to limit a spec run to individual examples or groups + # you care about by tagging them with `:focus` metadata. When nothing + # is tagged with `:focus`, all examples get run. RSpec also provides + # aliases for `it`, `describe`, and `context` that include `:focus` + # metadata: `fit`, `fdescribe` and `fcontext`, respectively. + config.filter_run_when_matching :focus + + # Allows RSpec to persist some state between runs in order to support + # the `--only-failures` and `--next-failure` CLI options. We recommend + # you configure your source control system to ignore this file. + config.example_status_persistence_file_path = "spec/examples.txt" + + # Limits the available syntax to the non-monkey patched syntax that is + # recommended. For more details, see: + # - http://rspec.info/blog/2012/06/rspecs-new-expectation-syntax/ + # - http://www.teaisaweso.me/blog/2013/05/27/rspecs-new-message-expectation-syntax/ + # - http://rspec.info/blog/2014/05/notable-changes-in-rspec-3/#zero-monkey-patching-mode + config.disable_monkey_patching! + + # This setting enables warnings. It's recommended, but in some cases may + # be too noisy due to issues in dependencies. + config.warnings = true + + # Many RSpec users commonly either run the entire suite or an individual + # file, and it's useful to allow more verbose output when running an + # individual spec file. + if config.files_to_run.one? + # Use the documentation formatter for detailed output, + # unless a formatter has already been configured + # (e.g. via a command-line flag). + config.default_formatter = 'doc' + end + + # Print the 10 slowest examples and example groups at the + # end of the spec run, to help surface which specs are running + # particularly slow. + config.profile_examples = 10 + + # Run specs in random order to surface order dependencies. If you find an + # order dependency and want to debug it, you can fix the order by providing + # the seed, which is printed after each run. + # --seed 1234 + config.order = :random + + # Seed global randomization in this process using the `--seed` CLI option. + # Setting this allows you to use `--seed` to deterministically reproduce + # test failures related to randomization by passing the same `--seed` value + # as the one that triggered the failure. + Kernel.srand config.seed +=end +end diff --git a/spec/task_spec.rb b/spec/task_spec.rb new file mode 100644 index 0000000..483853e --- /dev/null +++ b/spec/task_spec.rb @@ -0,0 +1,60 @@ +require_relative '../lib/model.rb' + +RSpec.describe Model do + describe "class PACTask" do + describe "method: new" do + context "When a PACTask is created" do + it "the data object should not be there" do + ta = Model::PACTask.new("1") + expect(ta.data).to be_nil + end + end + end + describe "applies_to" do + let(:ta) { Model::PACTask.new("2") } + let(:ta_full) { + m = Model::PACTask.new("2") + m.applies_to = 'jira' + m + } + + context "Given a new task" do + it "applies_to should be empty" do + expect(ta.applies_to.length).to be 0 + end + + it "should append the applied string when called" do + ta.applies_to = 'jira' + expect(ta.applies_to.length).to be 1 + end + + it "should append the new applied string to the end of the list when re-applied" do + ta_full.applies_to = 'trac' + expect(ta_full.applies_to.length).to be 2 + end + + end + end + + describe "add_label" do + let(:std_task) { Model::PACTask.new("1") } + context "Given a new task" do + it "label should be empty" do + expect(std_task.label.length).to be 0 + end + end + + context "When the user adds a label to the task" do + it "the label should be added to the list" do + end + end + end + end + describe "class: PACTaskCollection" do + context "Given a new collection" do + it "HAHA" do + expect(1).to be 1 + end + end + end +end diff --git a/test/unit/modeltest.rb b/test/unit/modeltest.rb index fdd034e..af7bc03 100644 --- a/test/unit/modeltest.rb +++ b/test/unit/modeltest.rb @@ -5,12 +5,42 @@ class ModelTest < Test::Unit::TestCase + + # Testing new PAC task creation + # but not the trivial get/setters + def test_PACTask_initialize + + # A PAC task initializes with a task_id, either nil or something that makese + # sense later in the system. This means a string. + tc = Model::PACTask.new() + assert_nil(tc.task_id,"Task initialized without task id should task_id nil") + + # Shouldn't reaise argument error as we expect nil or string + assert_nothing_raised(ArgumentError) { + tc = Model::PACTask.new('issuekey') + tc = Model::PACTask.new() + } + end + + + # Applies to method is setting the tasks systems the PAC tasks have data in + def test_PACtask_applies_to_method + ts = Model::PACTask.new() + + + assert_equal(Set.new,ts.applies_to(),"Applies to is default empty set.") + # Applies to take a string matching the entry for task system in the config file + ts.applies_to='mytasksystem' + tasksystem = Set.new ['mytasksystem'] + assert_equal(tasksystem,ts.applies_to(),"Applies to tasks system not 'mytasksystem' as expected.") + end + def test_model_task_collection tc = Model::PACTaskCollection.new - arr_of_tasks = [ Model::PACTask.new(1), Model::PACTask.new(2)] + arr_of_tasks = [ Model::PACTask.new("1"), Model::PACTask.new("2")] tc.add(arr_of_tasks) - ta = Model::PACTask.new(3) + ta = Model::PACTask.new("3") tc.add(ta) tc.add(ta) assert_true(tc.length == 3) @@ -21,10 +51,10 @@ def test_commit_collection_add commit1 = Model::PACCommit.new('abcd1234') commit1.referenced = true commit2 = Model::PACCommit.new('abcd1234abcd') - commit3 = Model::PACCommit.new('abcd12341235') + commit3 = Model::PACCommit.new('abcd12341235') cc.add(commit1) cc.add(commit2,commit3) - + assert_true(cc.count_with == 1) assert_true(cc.count_without == 2, "Commit list must contain 2 unreferenced commits, it contained #{cc.count_without}") assert_equal(33, (cc.health*100).to_i, "We expect the health to be 33, it was #{(cc.health*100).to_i}") @@ -38,7 +68,7 @@ def test_commit_associated_with_more_than_one_case task1 = Model::PACTask.new('task1') task1.add_commit(commit1) - + task11 = Model::PACTask.new('task1') task11.add_commit(commit2) @@ -59,13 +89,13 @@ def test_commit_associated_with_more_than_one_case assert_nil(tc['task4']) end - def test_decorator_module + def test_decorator_module task11 = Model::PACTask.new('STACI-2') #The collection of tasks is totally decoupled from the task system itself. Only requirement is that #you should be able to 'fetch' the required data from a task, given the id of the object. How that is done #is an implementaion detail. task_c = Model::PACTaskCollection.new - task_c.add(task11) + task_c.add(task11) assert_false(task11.attributes.has_key?('data')) task11.extend(JiraTaskDecorator) #Notice how that 'task11' now responds to the attributes call with the data field (empty at the moment) @@ -92,10 +122,10 @@ def test_grouping group2.add_commit(commit2) - + tc_grouped.add(group1) tc_grouped.add(group2) - + assert_equal(3, tc_grouped.by_label["STACI"].length) assert_equal(1, tc_grouped.by_label["LUCI"].length) @@ -104,4 +134,4 @@ def test_grouping end end -end \ No newline at end of file +end