Fixes tests on master #6

raychatter · 2018-02-01T01:46:01Z

What

Fixes rubocop failures using default rules with the exception of one
case
Removes CodeClimate from the code because its implementation was
deprecated on TravisCI. I opened an issue to look into this: Code Climate Integration #7
Added the latest stable ruby version (that RVM has to offer) to run on
TravisCI
Did the minimum necessary to mock out the repo linter functionality so
that the existing tests will pass. I will make a follow-up eventually
to write tests for the repo linter.
Fixed some small things in the existing tests to get them to pass
Added test coverage to the Runner spec.

raychatter · 2018-02-01T06:07:20Z

.rubocop.yml

@@ -26,6 +26,9 @@ Metrics/CyclomaticComplexity:
 Metrics/ParameterLists:
  Max: 10

+Style/AccessorMethodName:
+  Enabled: false
+


Because of the get_value method: https://github.com/thread-pond/scrum-lint/blob/master/lib/scrum_lint/reporters/interactive_reporter.rb#L10

raychatter · 2018-02-01T06:07:38Z

.travis.yml

@@ -4,6 +4,7 @@ rvm:
  - 2.1
  - 2.2
  - 2.3.0
+  - 2.4.3


Latest stable ruby RVM offers right now

raychatter · 2018-02-01T06:33:16Z

Gemfile

-
-group :test do
-  gem 'codeclimate-test-reporter'
-end


raychatter · 2018-02-01T06:37:46Z

lib/scrum_lint/configuration.rb

@@ -63,7 +63,7 @@ def config_file_contents
    end

    def config_file_path
-      './.scrum-lint.yml'
+      ENV.fetch('config_file_path', './.scrum-lint.yml')


Allows the user to specify where the .scrum-lint.yml file is located. This helps the test specs not use the real config file.

raychatter · 2018-02-01T06:38:30Z

lib/scrum_lint/interactive_linters/card/missing_checklist_item.rb

@@ -63,7 +63,7 @@ def lookup_project_card(context_link)
        ScrumLint::Trello::CardMapper.(
          trello_card,
          list: trello_card.list,
-          board_name: trello_card.board.name,
+          board_name: trello_card.board.name


RuboCop yelled: Style/TrailingCommaInArguments

so is the idea for this PR just to get everything passing, and then if we want to change the rubocop rules we can go back and do that in followups?

@kaifius Yup, exactly. I wanted to touch the minimum amount of things to get a passing build and then be more intentional in follow-ups. If you have some suggestions for things to improve, let me know!

cool, yeah, i think we should enable this rule and rename get_value to enable the other rule in followups. Or maybe even paste in synchroform's rubocop.yml and see what fails

raychatter · 2018-02-01T06:38:52Z

lib/scrum_lint/interactive_linters/card/missing_hash_tag.rb

@@ -3,7 +3,7 @@ module InteractiveLinter
    # checks that cards have #HashTag and interactively assigns
    class MissingHashTag < InteractiveLinter::Base

-      MESSAGE = 'missing hashtag'
+      MESSAGE = 'missing hashtag'.freeze


RuboCop yelled: Style/MutableConstant

raychatter · 2018-02-01T06:39:06Z

lib/scrum_lint/mappers/trello/list_mapper.rb

@@ -10,7 +10,7 @@ def call(trello_list, board_name:)
          list.cards = mapped_cards(
            list: list,
            trello_list: trello_list,
-            board_name: board_name,
+            board_name: board_name


RuboCop yelled: Style/TrailingCommaInArguments

raychatter · 2018-02-01T06:39:31Z

spec/fixtures/scrum-lint.yml

@@ -1,3 +1,4 @@
 trello_developer_public_key: nope
 trello_member_token: nada
 github_access_token: foo
+github_repo_names: ['foo/bar']


For the github repo mapper functionality that's now in configuration.rb

raychatter · 2018-02-01T06:43:01Z

spec/scrum_lint/configuration_spec.rb

-    it 'aborts when .scrum-lint.yml is missing' do
-      file_path = './.scrum-lint.yml'
+    it 'aborts when config file is missing' do
+      file_path = ENV.fetch('config_file_path')


Test modified to allow for configurable path to .scrum-lint.yml

raychatter · 2018-02-01T06:45:15Z

spec/scrum_lint/models/card_spec.rb

@@ -1,6 +1,7 @@
 RSpec.describe ScrumLint::Card do
  let(:card_params) do
    {
+      board_name: nil,


Required param for instantiating a ScrumLint::Card

raychatter · 2018-02-01T06:46:22Z

spec/scrum_lint/runner_spec.rb

@@ -3,7 +3,12 @@ module ScrumLint
    let(:runner) { described_class.new }

    before(:each) do
+      allow(Launchy).to receive(:open)


The stubbed objects may not be completely valid, so don't open Launchy at all

raychatter · 2018-02-01T06:47:29Z

spec/scrum_lint/runner_spec.rb

+        expect(Linter::MissingContext).not_to have_received(:call)
+        expect(Linter::MissingExpendedPoints).to have_received(:call)
+          .with(instance_of(Card))
+      end


Added code coverage! 🎉

raychatter · 2018-02-01T06:47:57Z

spec/scrum_lint/runner_spec.rb

      allow(::Trello::Board).to receive(:all).and_return([fake_trello_board])
+      allow(::Trello::Card).to receive(:find).and_return(fake_trello_card)
+
+      allow(Octokit::Source).to receive(:call).and_return([])


I'll deal with tests for the repo linter stuff later

raychatter · 2018-02-01T06:48:21Z

spec/scrum_lint/runner_spec.rb

      runner.([])
      expect(Linter::MissingTaskList).to have_received(:call)
        .with(instance_of(Board))
      expect(Linter::ExtraList).to have_received(:call).with(instance_of(Board))
+      expect(Linter::ExtraList).to have_received(:call).with(instance_of(Board))


Added code coverage! 🎉

does having this line here twice mean we expect this method to be called twice? if so i think you can do have_received(:call).twice.with(..)

Haha nope. It means I wasn't very careful with my copy/pasta-ing.

raychatter · 2018-02-01T06:53:50Z

spec/support/coverage.rb

-  else
-    SimpleCov.start
-  end
+  SimpleCov.start


raychatter · 2018-02-01T06:57:56Z

spec/support/fake_builders.rb

+    short_url: 'https://foo.bar.baz.biz',
+    url: 'https://something.much.longer',
+    **options
+  )


Attributes needed to properly instantiate a Card object

raychatter · 2018-02-01T06:58:11Z

spec/support/fake_builders.rb

 end

-def fake_trello_list(name: 'Planned', cards: [])
+def fake_trello_list(name: 'Planned', cards: [fake_trello_card])


Have at least one card in the list by default

kaifius

eeeeee exciting! just a couple lil comentarios

kaifius · 2018-02-01T23:24:36Z

lib/scrum_lint/interactive_linters/card/missing_checklist_item.rb

@@ -63,7 +63,7 @@ def lookup_project_card(context_link)
        ScrumLint::Trello::CardMapper.(
          trello_card,
          list: trello_card.list,
-          board_name: trello_card.board.name,
+          board_name: trello_card.board.name


so is the idea for this PR just to get everything passing, and then if we want to change the rubocop rules we can go back and do that in followups?

kaifius · 2018-02-01T23:36:17Z

spec/scrum_lint/runner_spec.rb

      runner.([])
      expect(Linter::MissingTaskList).to have_received(:call)
        .with(instance_of(Board))
      expect(Linter::ExtraList).to have_received(:call).with(instance_of(Board))
+      expect(Linter::ExtraList).to have_received(:call).with(instance_of(Board))


does having this line here twice mean we expect this method to be called twice? if so i think you can do have_received(:call).twice.with(..)

kaifius · 2018-02-01T23:37:50Z

spec/scrum_lint/runner_spec.rb

+        expect(Linter::MissingExpendedPoints).not_to have_received(:call)
+      end
+
+      it 'runs linters where card tags are superset of linter tags' do


can you update the it statement here to differentiate it from the one above?

Yeah, good catch. Being half asleep is probably not the best time for programming. I'll also add a little more coverage.

**What** - Fixes rubocop failures using default rules with the exception of one case - Removes CodeClimate from the code because its implementation was deprecated on TravisCI. I opened an issue to look into this: #7 - Added the latest stable ruby version (that RVM has to offer) to run on TravisCI - Did the minimum necessary to mock out the repo linter functionality so that the existing tests will pass. I will make a follow-up eventually to write tests for the repo linter. - Fixed some small things in the existing tests to get them to pass - Added test coverage to the `Runner` spec.

raychatter · 2018-02-02T00:44:54Z

Updated per comments.

raychatter force-pushed the fixin-failures branch 2 times, most recently from b1108e0 to b9cb493 Compare February 1, 2018 05:26

raychatter commented Feb 1, 2018

View reviewed changes

.travis.yml

@@ -4,6 +4,7 @@ rvm:

- 2.1

- 2.2

- 2.3.0

- 2.4.3

Copy link

Contributor Author

raychatter Feb 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest stable ruby RVM offers right now

raychatter commented Feb 1, 2018

View reviewed changes

Gemfile

group :test do

gem 'codeclimate-test-reporter'

end

Copy link

Contributor Author

raychatter Feb 1, 2018 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: #7

raychatter commented Feb 1, 2018

View reviewed changes

spec/support/coverage.rb

else

SimpleCov.start

end

SimpleCov.start

Copy link

Contributor Author

raychatter Feb 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: #7

raychatter commented Feb 1, 2018

View reviewed changes

raychatter force-pushed the fixin-failures branch from 81e9511 to 77048c2 Compare February 1, 2018 07:02

raychatter requested a review from kaifius February 1, 2018 07:10

raychatter assigned kaifius Feb 1, 2018

raychatter added the bug label Feb 1, 2018

raychatter force-pushed the fixin-failures branch from 77048c2 to e8d866a Compare February 1, 2018 07:21

raychatter changed the title ~~WIP: Making tests pass~~ Fixes tests on master Feb 1, 2018

kaifius reviewed Feb 1, 2018

View reviewed changes

kaifius assigned raychatter and unassigned kaifius Feb 1, 2018

kaifius added the pending comments label Feb 1, 2018

raychatter force-pushed the fixin-failures branch from e8d866a to 525e5b9 Compare February 2, 2018 00:43

raychatter removed the pending comments label Feb 2, 2018

raychatter assigned kaifius and unassigned raychatter Feb 2, 2018

kaifius merged commit 1fc1689 into master Feb 2, 2018

kaifius deleted the fixin-failures branch February 2, 2018 01:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes tests on master #6

Fixes tests on master #6

raychatter commented Feb 1, 2018 •

edited

Loading

raychatter Feb 1, 2018

raychatter Feb 1, 2018

raychatter Feb 1, 2018 •

edited

Loading

raychatter Feb 1, 2018

raychatter Feb 1, 2018

kaifius Feb 1, 2018

raychatter Feb 2, 2018

kaifius Feb 2, 2018

raychatter Feb 1, 2018

raychatter Feb 1, 2018

raychatter Feb 1, 2018 •

edited

Loading

raychatter Feb 1, 2018

raychatter Feb 1, 2018

raychatter Feb 1, 2018

raychatter Feb 1, 2018

raychatter Feb 1, 2018

raychatter Feb 1, 2018

kaifius Feb 1, 2018

raychatter Feb 2, 2018

raychatter Feb 1, 2018

raychatter Feb 1, 2018

raychatter Feb 1, 2018

kaifius left a comment

kaifius Feb 1, 2018

kaifius Feb 1, 2018

kaifius Feb 1, 2018

raychatter Feb 2, 2018

raychatter commented Feb 2, 2018

@@ @@ -4,6 +4,7 @@ rvm: @@
                 - 2.1
                 - 2.2
                 - 2.3.0
+                - 2.4.3

Fixes tests on master #6

Fixes tests on master #6

Conversation

raychatter commented Feb 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raychatter Feb 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raychatter Feb 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaifius left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raychatter commented Feb 2, 2018

raychatter commented Feb 1, 2018 •

edited

Loading

raychatter Feb 1, 2018 •

edited

Loading

raychatter Feb 1, 2018 •

edited

Loading