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.

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

Kintsugi: Add integration tests. #66

Merged
merged 2 commits into from
Jun 14, 2022

Conversation

byohay
Copy link
Collaborator

@byohay byohay commented May 30, 2022

  • kintsugi.gemspec: Add git as development dependency.
  • Add integration tests.

@byohay byohay force-pushed the feature/add-integration-tests branch from 3f50c2c to c5eed4d Compare May 30, 2022 14:38
@byohay byohay changed the title feature/add integration tests Kintsugi: Add integration tests. May 30, 2022
Copy link
Member

@ashdnazg ashdnazg left a comment

Choose a reason for hiding this comment

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

Looks good, you can rename lib/kintsugi/kintsugi.rb to lib/kintsugi/resolve.rb or similar.

kintsugi.gemspec Outdated
@@ -28,5 +28,7 @@ Gem::Specification.new do |spec|
spec.add_development_dependency "rspec", "~> 3.9"
spec.add_development_dependency "rubocop", "1.12.0"
spec.add_development_dependency "rubocop-rspec", "2.2.0"
spec.add_development_dependency "rubocop-performance", "1.13.3"
Copy link
Member

Choose a reason for hiding this comment

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

why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed.

Kintsugi::run([File.join(project.path, "project.pbxproj")])

project = Xcodeproj::Project.open(project_path)
expect(project.targets.count).to equal(2)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
expect(project.targets.count).to equal(2)
expect(project.targets.map(&:display_name)).to contain_exactly("foo", "bar")


# Changes are supposed to be in `ours_project` after driver is called.
ours_project = Xcodeproj::Project.open(ours_project.path)
expect(Xcodeproj::Project.open(ours_project.path).targets.count).to equal(2)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
expect(Xcodeproj::Project.open(ours_project.path).targets.count).to equal(2)
expect(ours_project.targets.map(&:display_name)).to contain_exactly("foo", "bar")

ours_project = Xcodeproj::Project.open(ours_project.path)
expect(Xcodeproj::Project.open(ours_project.path).targets.count).to equal(2)
end

Copy link
Member

Choose a reason for hiding this comment

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

add tests for errors - make sure the state of the folder in the end is what it should be.
If you can, also add tests for the driver

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a test that checks that after Kintsugi failed to resolve a conflict the project wasn't touched and is still has conflicts.

lib/kintsugi.rb Outdated
@@ -1,167 +1,49 @@
# Copyright (c) 2021 Lightricks. All rights reserved.
# Created by Ben Yohay.
#!/usr/bin/env ruby
Copy link
Member

Choose a reason for hiding this comment

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

this shouldn't be here

@byohay byohay marked this pull request as ready for review June 9, 2022 12:48
@byohay byohay force-pushed the feature/add-integration-tests branch 2 times, most recently from 2d97b16 to c026462 Compare June 9, 2022 12:50
@byohay
Copy link
Collaborator Author

byohay commented Jun 9, 2022

@ashdnazg round done.

@byohay byohay force-pushed the feature/add-integration-tests branch 8 times, most recently from d3aa28f to 4c29371 Compare June 9, 2022 15:09
Copy link
Member

@ashdnazg ashdnazg left a comment

Choose a reason for hiding this comment

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

One comment.
The rest is really great!

Kintsugi.run([File.join(project.path, "project.pbxproj")])
}.to raise_error(Kintsugi::MergeError)
expect {
project = Xcodeproj::Project.open(project.path)
Copy link
Member

Choose a reason for hiding this comment

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

is this the best way to check if conflicts are kept?
Maybe just diff the file with the previous version?

@byohay byohay force-pushed the feature/add-integration-tests branch from 4c29371 to b5b30e7 Compare June 13, 2022 14:27
@byohay byohay merged commit 56f6df0 into Lightricks:main Jun 14, 2022
@byohay byohay deleted the feature/add-integration-tests branch June 14, 2022 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants