Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor part 2 - separate merge from diff iterator [change-log] optimize merge by 20 percent #2884

Merged
merged 17 commits into from
Feb 6, 2022
Merged

Refactor part 2 - separate merge from diff iterator [change-log] optimize merge by 20 percent #2884

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8c0b07d
refactor - separate merge from dif iterator
eden-ohana Jan 23, 2022
0f0ee50
refactor - separate merge from dif iterator
eden-ohana Jan 23, 2022
5fabb17
refactor - separate merge from dif iterator
eden-ohana Jan 24, 2022
ddd1be7
refactor - separate merge from dif iterator
eden-ohana Jan 24, 2022
854e1ed
changes from PR
eden-ohana Jan 27, 2022
61e53ad
changes from PR
eden-ohana Jan 27, 2022
4e6cd9e
changes from PR
eden-ohana Jan 27, 2022
a11ba7d
changes from PR
eden-ohana Jan 27, 2022
bd975f9
changes from PR
eden-ohana Jan 30, 2022
9440d44
changes from PR
eden-ohana Jan 30, 2022
29cdb7d
changes from PR
eden-ohana Jan 30, 2022
24fbd6f
changes from PR
eden-ohana Jan 31, 2022
a4d6d0b
added tests to increase statements coverage
eden-ohana Jan 31, 2022
f1c7f86
added tests
eden-ohana Feb 2, 2022
9c1ef40
bug fix
eden-ohana Feb 3, 2022
18aea3f
added comments
eden-ohana Feb 6, 2022
10fa177
pr comments
eden-ohana Feb 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion nessie/identity_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestIdentity(t *testing.T) {

resp, err := client.MergeIntoBranchWithResponse(ctx, repo, branch1, branch2, api.MergeIntoBranchJSONRequestBody{})
require.NoError(t, err, "error during merge")
require.NotNil(t, resp.JSON400, "merge should fail since there are no changes between the branches")
require.Nil(t, resp.JSON400, "allow merge with no changes between the branches")
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a strange test to check specifically that the 400 error is empty - should probably check that the returned code is not empty (200?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

})
}
}
10 changes: 8 additions & 2 deletions pkg/api/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,10 @@ func TestController_CommitsGetBranchCommitLogByPath(t *testing.T) {
user: "user3",
commitName: "P",
})
mergeCommit, _ := deps.catalog.Merge(ctx, "repo3", "main", "branch-b", "user3", "commitR", nil)
mergeCommit, err := deps.catalog.Merge(ctx, "repo3", "main", "branch-b", "user3", "commitR", nil)
if err != nil {
t.Fatal(err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if err != nil {
t.Fatal(err)
}
testutil.Must(t, err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

commitsMap["commitR"] = mergeCommit.Reference
commitsMap["commitM"] = testCommitEntries(t, ctx, deps.catalog, deps, commitEntriesParams{
repo: "repo3",
Expand All @@ -352,7 +355,10 @@ func TestController_CommitsGetBranchCommitLogByPath(t *testing.T) {
user: "user2",
commitName: "M",
})
mergeCommit, _ = deps.catalog.Merge(ctx, "repo3", "main", "branch-a", "user2", "commitN", nil)
mergeCommit, err = deps.catalog.Merge(ctx, "repo3", "main", "branch-a", "user2", "commitN", nil)
if err != nil {
t.Fatal(err)
}
commitsMap["commitN"] = mergeCommit.Reference
commitsMap["commitX"] = testCommitEntries(t, ctx, deps.catalog, deps, commitEntriesParams{
repo: "repo3",
Expand Down
Loading