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

Use test-unit gem instead of test-framework of ruby repo #216

Merged
merged 11 commits into from
Jun 4, 2021
Merged

Conversation

hsbt
Copy link
Member

@hsbt hsbt commented Jun 1, 2021

I replaced test-framework provided by ruby/ruby repo to test-unit gem and the additional assertions named CoreAssertions.

This change makes unifying to assertion verb with test-unit gem.

@kou
Copy link
Member

kou commented Jun 1, 2021

Thanks.
I'll add some methods to test-unit gem to reduce more files.

@hsbt
Copy link
Member Author

hsbt commented Jun 2, 2021

@kou What can I do the next step? If you are okay to this, I will merge this.

I'll add some methods to test-unit gem to reduce more files.

It's great for me.

@kou
Copy link
Member

kou commented Jun 2, 2021

Could you wait for a few days? I want to review all changes and try this on local. For example, I think that we should not remove is_output_field_separator_deprecated check in test/lib/with_different_ofs.rb. (I think that we need to check it with $VERBOSE = true.)

@hsbt
Copy link
Member Author

hsbt commented Jun 3, 2021

Could you wait for a few days?

It's ok, no problem.

I think that we should not remove is_output_field_separator_deprecated check in test/lib/with_different_ofs.rb

👍 I picked this helper from ruby/ruby repo originally. It's better to adjust to CSV library.

@kou
Copy link
Member

kou commented Jun 4, 2021

  • Fixed WithDifferentOFS.
  • Removed CoreAssertions entirely by adding needed assertions to test-unit.

@kou kou merged commit 9c4add0 into master Jun 4, 2021
@kou kou deleted the test-unit branch June 4, 2021 08:11
@hsbt
Copy link
Member Author

hsbt commented Jun 4, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants