Circular dependencies #4

soonernotfaster · 2017-08-20T00:06:41Z

In this branch I have done the following:

Added basic test coverage through the Data class using RSpec
Implemented the feature
Added example file in lib to test behavior.

I am happy to discuss switching to another testing framework if you prefer, I simply chose what I am most comfortable with. Also, I am hoping for a better way to test the app then pollute it with the test classes.
Fixes #3

- Remove test duplication

- Move definitions and relations parsing into parse method - Test app from Data class rather than File class - Add example circular file to test

houndci-bot · 2017-08-20T00:06:54Z

spec/parser/fixtures/class_with_one_included_mixin.rb

@@ -0,0 +1,3 @@
+class ClassWithOneIncludedMixin
+  include ModuleWithNoRelations
+end


Final newline missing.

houndci-bot · 2017-08-20T00:06:54Z

spec/parser/fixtures/class_with_one_included_mixin.rb

@@ -0,0 +1,3 @@
+class ClassWithOneIncludedMixin


Missing magic comment # frozen_string_literal: true.

houndci-bot · 2017-08-20T00:06:54Z

spec/spec_helper.rb

+
+# The settings below are suggested to provide a good initial experience
+# with RSpec, but feel free to customize to your heart's content.
+=begin


Do not use block comments.

houndci-bot · 2017-08-20T00:06:54Z

spec/spec_helper.rb

@@ -0,0 +1,100 @@
+# This file was generated by the `rspec --init` command. Conventionally, all


Missing magic comment # frozen_string_literal: true.

houndci-bot · 2017-08-20T00:06:55Z

spec/parser/fixtures/module_with_no_relations.rb

@@ -0,0 +1,2 @@
+module ModuleWithNoRelations
+end


Final newline missing.

houndci-bot · 2017-08-20T00:06:55Z

spec/parser/fixtures/classes_circular_dependency.rb

@@ -0,0 +1,17 @@
+class A


Missing magic comment # frozen_string_literal: true.

houndci-bot · 2017-08-20T00:06:55Z

spec/parser/fixtures/class_with_one_extended_mixin.rb

@@ -0,0 +1,3 @@
+class ClassWithOneExtendedMixin
+  extend ModuleWithNoRelations
+end


Final newline missing.

houndci-bot · 2017-08-20T00:06:55Z

spec/parser/fixtures/class_with_one_extended_mixin.rb

@@ -0,0 +1,3 @@
+class ClassWithOneExtendedMixin


Missing magic comment # frozen_string_literal: true.

houndci-bot · 2017-08-20T00:06:55Z

spec/parser/data_spec.rb

+      end
+    end
+  end
+end


Final newline missing.

houndci-bot · 2017-08-20T00:06:55Z

spec/parser/data_spec.rb

+
+  context 'Circular dependency' do
+    context 'across classes' do
+      let(:file_path) {'spec/parser/fixtures/classes_circular_dependency.rb'}


Space missing inside {.
Space missing inside }.

houndci-bot · 2017-08-20T00:16:44Z

spec/parser/fixtures/class_with_one_class_dependency.rb

@@ -0,0 +1,5 @@
+class ClassWithOneClassDependency


Missing magic comment # frozen_string_literal: true.

houndci-bot · 2017-08-20T00:16:45Z

spec/parser/fixtures/class_with_no_relations.rb

@@ -0,0 +1,2 @@
+class ClassWithNoRelations


Missing magic comment # frozen_string_literal: true.

houndci-bot · 2017-08-20T00:16:45Z

spec/parser/data_spec.rb

+  context 'Class depends on another class' do
+    let(:file_namespace) {:ClassWithOneClassDependency}
+    let(:dependency_namespace) {:ClassWithNoRelations}
+    let(:file_path) { 'spec/parser/fixtures/class_with_one_class_dependency.rb' }


Line is too long. [81/80]

houndci-bot · 2017-08-20T00:16:45Z

spec/parser/data_spec.rb

+
+  context 'Class depends on another class' do
+    let(:file_namespace) {:ClassWithOneClassDependency}
+    let(:dependency_namespace) {:ClassWithNoRelations}


Space missing inside {.
Space missing inside }.

houndci-bot · 2017-08-20T00:16:45Z

spec/parser/data_spec.rb

+  end
+
+  context 'Class depends on another class' do
+    let(:file_namespace) {:ClassWithOneClassDependency}


Space missing inside {.
Space missing inside }.

houndci-bot · 2017-08-20T00:16:45Z

spec/parser/data_spec.rb

+    context 'extended' do
+      let(:file_namespace) {:ClassWithOneExtendedMixin}
+      let(:dependency_namespace) {:ModuleWithNoRelations}
+      let(:file_path) { 'spec/parser/fixtures/class_with_one_extended_mixin.rb' }


Line is too long. [81/80]

houndci-bot · 2017-08-20T00:16:45Z

spec/parser/data_spec.rb

+
+    context 'extended' do
+      let(:file_namespace) {:ClassWithOneExtendedMixin}
+      let(:dependency_namespace) {:ModuleWithNoRelations}


Space missing inside {.
Space missing inside }.

houndci-bot · 2017-08-20T00:24:38Z

spec/spec_helper.rb

@@ -0,0 +1,101 @@
+# frozen_string_literal: false
+# This file was generated by the `rspec --init` command. Conventionally, all


Add an empty line after magic comments.

soonernotfaster · 2017-08-20T00:27:28Z

Is there a way to run these checks locally? I don't want to have a million tiny commits.

emad-elsaid · 2017-08-20T03:20:30Z

@steven-solomon Yup, try rubocop locally I think houndci is just running that command

houndci-bot · 2017-08-20T04:15:45Z

spec/parser/data_spec.rb

@@ -0,0 +1,116 @@
+require 'rubrowser/data'
+
+describe Rubrowser::Data do


Block has too many lines. [90/25]

Can we make an exception to this rule for spec files?

houndci-bot · 2017-08-20T04:15:45Z

spec/parser/data_spec.rb

@@ -0,0 +1,116 @@
+require 'rubrowser/data'


Missing magic comment # frozen_string_literal: true.

houndci-bot · 2017-08-20T04:15:45Z

public/javascript/application.js

@@ -4,6 +4,10 @@ d3.json("/data.json", function(error, data) {
  $('.toolbox').show();
 });

+var classForCircular = function(d) {
+  return d.circular ? 'circular' : ''
+}


Missing semicolon.

houndci-bot · 2017-08-20T04:15:45Z

public/javascript/application.js

@@ -4,6 +4,10 @@ d3.json("/data.json", function(error, data) {
  $('.toolbox').show();
 });

+var classForCircular = function(d) {
+  return d.circular ? 'circular' : ''


Missing semicolon.

houndci-bot · 2017-08-20T04:15:45Z

lib/rubrowser/data.rb

    end

    private

+    class Graph < Hash


Missing top-level class documentation comment.

houndci-bot · 2017-08-20T04:15:45Z

lib/rubrowser/data.rb

+      end
+
+      @relations.each do |relation|
+        if components.include?(relation.namespace.to_s)


Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.

In this case, I feel that using &&/|| for control flow would make the code unclear. How do you feel?

houndci-bot · 2017-08-20T04:15:46Z

lib/rubrowser/a.rb

+  def self.do; end
+end
+
+class C


Missing top-level class documentation comment.

houndci-bot · 2017-08-20T04:15:46Z

lib/rubrowser/a.rb

+  def self.do; end
+end
+
+class B


Missing top-level class documentation comment.

I'd love to find another way to test the app's circular functionality without including a file that does nothing

houndci-bot · 2017-08-20T04:31:07Z

lib/rubrowser/a.rb

@@ -0,0 +1,23 @@
+class A


Missing top-level class documentation comment.
Missing magic comment # frozen_string_literal: true.

soonernotfaster

Mostly there. I would like to propose an exception to the block rule on test files and comments for classes as no other class in the system has comments like this.

emad-elsaid · 2017-08-20T17:19:10Z

That's a good idea, lets add these exceptions to rubocop config file

houndci-bot · 2017-08-20T20:02:31Z

spec/parser/fixtures/module_with_no_relations.rb

@@ -0,0 +1,3 @@
+module ModuleWithNoRelations
+end
+


1 trailing blank lines detected.

houndci-bot · 2017-08-20T20:02:32Z

spec/parser/fixtures/class_with_one_included_mixin.rb

+class ClassWithOneIncludedMixin
+  include ModuleWithNoRelations
+end
+


1 trailing blank lines detected.

houndci-bot · 2017-08-20T20:02:32Z

spec/parser/fixtures/classes_circular_dependency.rb

+
+  def self.do; end
+end
+


1 trailing blank lines detected.

houndci-bot · 2017-08-20T20:02:32Z

spec/parser/fixtures/class_with_one_extended_mixin.rb

+class ClassWithOneExtendedMixin
+  extend ModuleWithNoRelations
+end
+


1 trailing blank lines detected.

houndci-bot · 2017-08-20T20:02:32Z

spec/parser/fixtures/module_with_one_class_dependency.rb

+    ClassWithNoRelations.new
+  end
+end
+


1 trailing blank lines detected.

houndci-bot · 2017-08-20T20:02:32Z

spec/parser/fixtures/module_with_one_class_dependency.rb

@@ -0,0 +1,6 @@
+class ModuleWithOneClassDependency


Carriage return character detected.
Missing magic comment # frozen_string_literal: true.

houndci-bot · 2017-08-20T20:02:32Z

spec/parser/fixtures/class_with_one_class_dependency.rb

+    ClassWithNoRelations.new
+  end
+end
+


1 trailing blank lines detected.

houndci-bot · 2017-08-20T20:02:32Z

spec/parser/fixtures/class_with_no_relations.rb

@@ -0,0 +1,3 @@
+class ClassWithNoRelations
+end
+


1 trailing blank lines detected.

soonernotfaster · 2017-08-20T21:54:03Z

Seems like houndci is ignoring the changes to .rubocop.yml. I'm going to close and reopen this pull request.

Steven Solomon added 6 commits August 19, 2017 11:39

Add RSpec to project

da30e67

Add rake task to start server

1321398

Add byebug

85658c8

Add test coverage for file

6039ca5

Add coverage for Module that depends on class

96ef96b

- Remove test duplication

Implement checks for circular relations

8f951f8

- Move definitions and relations parsing into parse method - Test app from Data class rather than File class - Add example circular file to test

houndci-bot reviewed Aug 20, 2017

View reviewed changes

Fix style issues in Ruby code

ef9dbde

houndci-bot reviewed Aug 20, 2017

View reviewed changes

Add magical comment

5da88e5

houndci-bot reviewed Aug 20, 2017

View reviewed changes

Style fixes to code

9f4bcbf

houndci-bot reviewed Aug 20, 2017

View reviewed changes

Add semicolon to method

973e4f2

houndci-bot reviewed Aug 20, 2017

View reviewed changes

soonernotfaster commented Aug 20, 2017

View reviewed changes

Add exceptions for Rubocop to ignore test length

a77a93e

soonernotfaster force-pushed the master branch from 4394a53 to a77a93e Compare August 20, 2017 19:52

houndci-bot reviewed Aug 20, 2017

View reviewed changes

soonernotfaster force-pushed the master branch from cb3b94e to a77a93e Compare August 20, 2017 20:04

soonernotfaster closed this Aug 20, 2017

soonernotfaster mentioned this pull request Aug 20, 2017

Circular dependencies #4 #5

Closed

		@@ -0,0 +1,100 @@
		# This file was generated by the `rspec --init` command. Conventionally, all

		@@ -0,0 +1,101 @@
		# frozen_string_literal: false
		# This file was generated by the `rspec --init` command. Conventionally, all

		@@ -0,0 +1,116 @@
		require 'rubrowser/data'

		describe Rubrowser::Data do

Circular dependencies #4

Circular dependencies #4

Conversation

soonernotfaster commented Aug 20, 2017

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

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

soonernotfaster commented Aug 20, 2017

emad-elsaid commented Aug 20, 2017

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

Choose a reason for hiding this comment

soonernotfaster left a comment

Choose a reason for hiding this comment

emad-elsaid commented Aug 20, 2017

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

soonernotfaster commented Aug 20, 2017