[Issue#9] - internal/graph: partition algorithm #16

mccurdyc · 2019-10-15T13:04:11Z

Proposed Changes:

Relevant Issue(s)/Link(s):

Fixes #

Describe Your Testing Method(s):

go test ./...

Risks/Caveats:

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph/graph.go

…connectedness Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph/graph.go

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph/graph.go

… distance Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph/graph.go

…global) vs just min (local) of connectedness value Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

…ds to funcs Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

… against edge distance Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot · 2019-10-23T11:00:50Z

internal/graph/graph_test.go

+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := test.graph.FindRoots()


Using the variable on range scope test in function literal (from scopelint)

golangcibot · 2019-10-23T11:00:50Z

internal/graph/graph_test.go

+		t.Run(test.name, func(t *testing.T) {
+			got := test.graph.FindRoots()
+
+			if diff := cmp.Diff(test.want, got); diff != "" {


Using the variable on range scope test in function literal (from scopelint)

golangcibot · 2019-10-23T11:00:51Z

internal/graph/graph_test.go

+			got := test.graph.FindRoots()
+
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("(%+v) FindRoots() mismatch (-want +got): \n%s", test.graph, diff)


Using the variable on range scope test in function literal (from scopelint)

golangcibot · 2019-10-23T11:00:51Z

internal/graph/graph_test.go

 }

 var (
+	parentEdge = map[string]WeightedEdge{


parentEdge is a global variable (from gochecknoglobals)

golangcibot · 2019-10-23T11:00:51Z

internal/graph/graph_test.go


-	nodeAWithSingleEdgeB = Node{ID: "a", Edges: edgeB}
-	nodeCWithSingleEdgeB = Node{ID: "c", Edges: edgeB}
+	nodeAWithSingleEdgeB    = Node{ID: "a", Edges: edgeB}


nodeAWithSingleEdgeB is a global variable (from gochecknoglobals)

golangcibot · 2019-10-23T11:00:51Z

internal/graph/graph_test.go

-	nodeAWithSingleEdgeB = Node{ID: "a", Edges: edgeB}
-	nodeCWithSingleEdgeB = Node{ID: "c", Edges: edgeB}
+	nodeAWithSingleEdgeB    = Node{ID: "a", Edges: edgeB}
+	nodeCWithSingleEdgeB    = Node{ID: "c", Edges: edgeB}


nodeCWithSingleEdgeB is a global variable (from gochecknoglobals)

golangcibot · 2019-10-23T11:00:52Z

internal/graph/graph_test.go

-	nodeCWithSingleEdgeB = Node{ID: "c", Edges: edgeB}
+	nodeAWithSingleEdgeB    = Node{ID: "a", Edges: edgeB}
+	nodeCWithSingleEdgeB    = Node{ID: "c", Edges: edgeB}
+	nodeCWithEdgeBAndParent = Node{ID: "c", Edges: edgeB, Parents: parentEdge}


nodeCWithEdgeBAndParent is a global variable (from gochecknoglobals)

golangcibot · 2019-10-23T11:00:52Z

internal/graph/node_test.go

 )

+var (
+	nodeA = Node{


nodeA is a global variable (from gochecknoglobals)

golangcibot · 2019-10-23T11:00:52Z

internal/graph/node_test.go

+		ShortestPathLen: 1.0,
+	}
+
+	nodeB = Node{


nodeB is a global variable (from gochecknoglobals)

…Issue#9/partition-algorithm

mccurdyc · 2019-11-07T12:21:04Z

internal/graph/graph.go

+func (g Graph) FindRoots() []*Node {
+	roots := make([]*Node, 0, len(g))
+
+	for _, node := range g {


order matters here

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot · 2019-11-29T20:32:27Z

ider_test.go

 				pkgFn:  tt.pkgFn,
 				nameFn: tt.nameFn,
-				posFn:  tt.posFn,
+				typeFn: tt.typeFn,


Using the variable on range scope tt in function literal (from scopelint)

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot · 2019-12-03T13:30:12Z

internal/graph/graph_test.go

+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := test.graph.Roots()


Using the variable on range scope test in function literal (from scopelint)

golangcibot · 2019-12-03T13:30:12Z

internal/graph/graph_test.go

+			got := test.graph.Roots()
+
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("(%+v) Roots() mismatch (-want +got): \n%s", test.graph, diff)


Using the variable on range scope test in function literal (from scopelint)

golangcibot · 2019-12-03T13:30:13Z

internal/graph/node_test.go


 			if ok := reflect.DeepEqual(*tt.node, tt.want); !ok {
-				t.Errorf("(%+v) AddEdge(%+v, %f) - mismatch \n%+v", tt.node, tt.dest, tt.weight, tt.want)
+				t.Errorf("AddEdge() - mismatch \n\twant: %+v\n\tgot:%+v", tt.want, *tt.node)


Using the variable on range scope tt in function literal (from scopelint)

golangcibot · 2019-12-03T13:30:13Z

internal/graph/graph_test.go

 }

 var (
+	parentNode = Node{ID: "parent", Edges: make(map[string]WeightedEdge), Parents: make(map[string]WeightedEdge)}


parentNode is a global variable (from gochecknoglobals)

golangcibot · 2019-12-03T13:30:13Z

internal/graph/graph_test.go


-	nodeAWithSingleEdgeB = Node{ID: "a", Edges: edgeB}
-	nodeCWithSingleEdgeB = Node{ID: "c", Edges: edgeB}
+	nodeAWithSingleEdgeB    = Node{ID: "a", Edges: edgeB, Parents: make(map[string]WeightedEdge)}


nodeAWithSingleEdgeB is a global variable (from gochecknoglobals)

golangcibot · 2019-12-03T13:30:13Z

internal/graph/graph_test.go

-	nodeAWithSingleEdgeB = Node{ID: "a", Edges: edgeB}
-	nodeCWithSingleEdgeB = Node{ID: "c", Edges: edgeB}
+	nodeAWithSingleEdgeB    = Node{ID: "a", Edges: edgeB, Parents: make(map[string]WeightedEdge)}
+	nodeCWithSingleEdgeB    = Node{ID: "c", Edges: edgeB, Parents: make(map[string]WeightedEdge)}


nodeCWithSingleEdgeB is a global variable (from gochecknoglobals)

golangcibot · 2019-12-03T13:30:14Z

splitfile_test.go

+)
+
+var (
+	emptyNode = graph.Node{


emptyNode is a global variable (from gochecknoglobals)

…ren't added Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

…eing evaluated. this race condition lead to missing edges if method nodes were added first Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

… terminate recursion! It completes now Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

… partitions Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

…owest). add a log message and avoid division by 0 Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot · 2019-12-10T13:56:07Z

internal/graph/partition.go

+
+// recursiveBFS does recursive breadth-first search of the graph, keeping track
+// of shortest path and the mininimum path strength (i.e., the weaked edge in a path).
+func recursiveBFS(node *Node, visited map[string]bool) {


cyclomatic complexity 12 of func recursiveBFS is high (> 10) (from gocyclo)

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot · 2019-12-11T14:06:31Z

internal/graph/partition.go

+
+// recursiveBFS does recursive breadth-first search of the graph, keeping track
+// of shortest path and the mininimum path strength (i.e., the weaked edge in a path).
+func recursiveBFS(level map[string]*Node, visited map[string]bool) {


cyclomatic complexity 17 of func recursiveBFS is high (> 10) (from gocyclo)

…or the same node Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

Calculate shortest paths of children on the way "down" the graph. Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

* removed queueing of nodes in bfs * returned a value `changed` from shortestPath function to indicate whether the value returned is a new shortest or not. * added tests for bfs and updated tests for shortestPath Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

* remove deprecated fields for MinPathStrength * add field for keeping track of shortest paths distances to a node Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot · 2019-12-17T12:20:49Z

internal/graph/partition_test.go

+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			got := bfs(tt.input)


Using the variable on range scope tt in function literal (from scopelint)

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot · 2019-12-17T12:35:44Z

internal/graph/partition_test.go

+		t.Run(name, func(t *testing.T) {
+			got := bfs(tt.input)
+
+			if diff := cmp.Diff(got, tt.want); diff != "" {


Using the variable on range scope tt in function literal (from scopelint)

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

qlty-cloud-legacy · 2019-12-18T13:23:19Z

internal/graph/partition_test.go

+	}
+}
+
+func Test_shortestPath(t *testing.T) {


Function Test_shortestPath has 113 lines of code (exceeds 25 allowed). Consider refactoring.

qlty-cloud-legacy · 2019-12-18T13:23:19Z

internal/graph/partition.go

+}
+
+// bfs does breadth-first search of the graph, keeping track of the current and past shortest paths.
+func bfs(level map[string]*Node) map[string]*Node {


Function bfs has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.

qlty-cloud-legacy · 2019-12-18T13:23:19Z

internal/graph/partition_test.go

+	"github.com/google/go-cmp/cmp"
+)
+
+func Test_bfs(t *testing.T) {


Function Test_bfs has 46 lines of code (exceeds 25 allowed). Consider refactoring.

qlty-cloud-legacy · 2019-12-18T13:23:19Z

internal/graph/graph_test.go

 	}
 }
+
+func TestRoots(t *testing.T) {


Function TestRoots has 27 lines of code (exceeds 25 allowed). Consider refactoring.

qlty-cloud-legacy · 2019-12-18T13:23:19Z

internal/graph/partition_test.go

+	}
+}
+
+func Test_shortestPath(t *testing.T) {


Function Test_shortestPath has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.

qlty-cloud-legacy · 2019-12-18T13:23:22Z

Code Climate has analyzed commit f1e2a9c and detected 11 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	7
Bug Risk	4

The test coverage on the diff in this pull request is 56.7% (50% is the threshold).

This pull request will bring the total coverage in the repository to 66.2%.

View more on Code Climate.

codecov-io · 2019-12-18T13:25:46Z

Codecov Report

❗ No coverage uploaded for pull request base (master@e510e09). Click here to learn what that means.
The diff coverage is 38.75%.

@@            Coverage Diff            @@
##             master      #16   +/-   ##
=========================================
  Coverage          ?   62.38%           
=========================================
  Files             ?        5           
  Lines             ?      226           
  Branches          ?        0           
=========================================
  Hits              ?      141           
  Misses            ?       73           
  Partials          ?       12

internal/graph: work in progress on partition algorithm

e967317

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot reviewed Oct 15, 2019

View reviewed changes

internal/graph/graph.go Outdated Show resolved Hide resolved

internal/graph/graph.go Outdated Show resolved Hide resolved

internal/graph/graph.go Outdated Show resolved Hide resolved

internal/graph/graph.go Outdated Show resolved Hide resolved

[WIP] internal/graph: working on recursiveBFS and the calculation of …

f75e31b

…connectedness Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot reviewed Oct 16, 2019

View reviewed changes

internal/graph/graph.go Outdated Show resolved Hide resolved

[WIP] internal/graph: working on calculating edge distances

037f784

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot reviewed Oct 17, 2019

View reviewed changes

internal/graph/graph.go Outdated Show resolved Hide resolved

internal/graph/graph.go Outdated Show resolved Hide resolved

[WIP] internal/graph: graph traversal is done. working on calculating…

a869368

… distance Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot reviewed Oct 18, 2019

View reviewed changes

internal/graph/graph.go Outdated Show resolved Hide resolved

internal/graph/graph.go Outdated Show resolved Hide resolved

internal/graph/graph.go Outdated Show resolved Hide resolved

mccurdyc added 7 commits October 21, 2019 08:53

internal/graph: first full draft complete. thinking about summation (…

01264a6

…global) vs just min (local) of connectedness value Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph: moving parition code to its own file and moving metho…

7b79d8c

…ds to funcs Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

splitfile: update to use the new Partition API

f783132

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph: update partition identifier function to check epsilon…

5bc520d

… against edge distance Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

[WIP] internal/graph: update graph tests to work with new API

afd6b38

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

splitfile: update epsilon value 1 -> 0.1

1ba49f1

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

testdata/src/abc: update testdata to use example in the README

2c24453

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot reviewed Oct 23, 2019

View reviewed changes

Merge branch 'master' of github.com:mccurdyc/splitfile into mccurdyc/…

4f11189

…Issue#9/partition-algorithm

mccurdyc commented Nov 7, 2019

View reviewed changes

ider: id function uses type instead of position and add tests

bfd037f

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot reviewed Nov 29, 2019

View reviewed changes

mccurdyc added 10 commits November 30, 2019 10:19

checkSignature should not return the receiver variable as a related node

d0aaee7

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

checkSignature should not return the receiver variable as a related node

e83341a

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph: add methods for returning Roots and Edges of a graph

cb8463e

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph: add source and destination to edges

57eef69

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph: partition should use new Roots and Edges methods

b019b87

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

analysis/test: uncomment test from filesystem

ac55b8d

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

add gitignore to ignore testdata/pkg

776af2d

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph: add placeholder for partition tests

37876b2

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

splitfile tests

f75ebe4

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

WIP; working on graph traversal. not working properly

b5a786a

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot reviewed Dec 3, 2019

View reviewed changes

mccurdyc added 7 commits December 4, 2019 08:00

WIP; fixed method related. there is still an issue where some edges a…

1007883

…ren't added Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

fix race condition between method node being added first and method b…

1b51055

…eing evaluated. this race condition lead to missing edges if method nodes were added first Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

recursiveBFS: needed to remove the element from the queue in order to…

d9dc68c

… terminate recursion! It completes now Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

add debug logging to give additional information about the identified…

6d19fec

… partitions Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

set and use defaultWeight of -1

d32bf9c

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

WIP; check for default values to avoid setting weights to defaults (l…

786e562

…owest). add a log message and avoid division by 0 Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

WIP; fix weights. lower inidicates closer

f6aae20

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot reviewed Dec 10, 2019

View reviewed changes

mccurdyc added 2 commits December 11, 2019 07:05

dont add default values to minpathweights

0333162

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

WIP; refactor recursiveBFS; builds, but havent verified correctness

448de64

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot reviewed Dec 11, 2019

View reviewed changes

mccurdyc added 4 commits December 12, 2019 08:07

fix reference bug where edges could be referencing different values f…

80418b2

…or the same node Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph: simplify recursiveBFS function

4f34901

Calculate shortest paths of children on the way "down" the graph. Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

internal/graph/node: use -1.0 as defaultWeight

f24be6a

* remove deprecated fields for MinPathStrength * add field for keeping track of shortest paths distances to a node Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot reviewed Dec 17, 2019

View reviewed changes

internal/graph/partition: update bfs comment

3ae6e82

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

golangcibot reviewed Dec 17, 2019

View reviewed changes

WIP; need to write unit tests...enough of this manual checking

f1e2a9c

Signed-off-by: Colton McCurdy <mccurdyc22@gmail.com>

qlty-cloud-legacy bot reviewed Dec 18, 2019

View reviewed changes

[Issue#9] - internal/graph: partition algorithm #16

Are you sure you want to change the base?

[Issue#9] - internal/graph: partition algorithm #16

Uh oh!

Conversation

mccurdyc commented Oct 15, 2019

Proposed Changes:

Relevant Issue(s)/Link(s):

Describe Your Testing Method(s):

Risks/Caveats:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy bot Dec 18, 2019

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy bot Dec 18, 2019

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy bot Dec 18, 2019

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy bot Dec 18, 2019

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy bot Dec 18, 2019

Choose a reason for hiding this comment

Uh oh!