From b44eaf8e0772d4ab076bd0df10b13085483e5e66 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 3 May 2017 09:29:49 +0000 Subject: Sort the network graph both by commit date and topographically. - Previously, we sorted commits by date, which seemed to work okay. - The one edge case where this failed was when multiple commits have the same commit date (for example: when a range of commits are cherry picked with a single command, they all have the same commit date [and different author dates]). - Commits with the same commit date would be sorted arbitrarily, and usually break the network graph. - This commit solves the problem by both sorting by date, and by sorting topographically (parents aren't displayed until all their children are displayed) - Include review comments from @adamniedzielski A more detailed explanation is present here: https://gitlab.com/gitlab-org/gitlab-ce/issues/30973#note_28706230 --- spec/models/network/graph_spec.rb | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) (limited to 'spec/models/network') diff --git a/spec/models/network/graph_spec.rb b/spec/models/network/graph_spec.rb index 46b36e11c23..0fe8a591a45 100644 --- a/spec/models/network/graph_spec.rb +++ b/spec/models/network/graph_spec.rb @@ -10,17 +10,17 @@ describe Network::Graph, models: true do expect(graph.notes).to eq( { note_on_commit.commit_id => 1 } ) end - describe "#commits" do + describe '#commits' do let(:graph) { described_class.new(project, 'refs/heads/master', project.repository.commit, nil) } - it "returns a list of commits" do + it 'returns a list of commits' do commits = graph.commits expect(commits).not_to be_empty expect(commits).to all( be_kind_of(Network::Commit) ) end - it "sorts the commits by commit date (descending)" do + it 'it the commits by commit date (descending)' do # Remove duplicate timestamps because they make it harder to # assert that the commits are sorted as expected. commits = graph.commits.uniq(&:date) @@ -29,5 +29,20 @@ describe Network::Graph, models: true do expect(commits).not_to be_empty expect(commits.map(&:id)).to eq(sorted_commits.map(&:id)) end + + it 'sorts children before parents for commits with the same timestamp' do + commits_by_time = graph.commits.group_by(&:date) + + commits_by_time.each do |time, commits| + commit_ids = commits.map(&:id) + + commits.each_with_index do |commit, index| + parent_indexes = commit.parent_ids.map { |parent_id| commit_ids.find_index(parent_id) }.compact + + # All parents of the current commit should appear after it + expect(parent_indexes).to all( be > index ) + end + end + end end end -- cgit v1.2.1