-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add unit test #22
Add unit test #22
Conversation
495fb4c
to
93f8d2b
Compare
13b4c0d
to
10a68e8
Compare
534f13f
to
51872bc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tests Sofia!
Please don't hesitate to ping us for more reviews.
Rakefile
Outdated
@@ -8,3 +8,9 @@ Yast::Tasks.configuration do |conf| | |||
conf.install_locations["desktop/*"] = File.join(Packaging::Configuration::DESTDIR, "/usr/share/applications/") | |||
conf.install_locations["mime/*"] = File.join(Packaging::Configuration::DESTDIR, "/usr/share/mime/packages/") | |||
end | |||
|
|||
# This is required, because `yast-travis-ruby` binary calls `rake test:unit` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rake -W
will show that "test:unit" is already defined in https://github.com/yast/yast-rake/blob/613cdb654d963a9acd8e35439ddaa1125857345c/lib/tasks/test_unit.rake#L54
The catch is that for historical reasons it expects the directory to be named test
, not spec
.
Until yast-rake resolves this it would be probably best to symlink test
-> spec
here and remove this task, then both rspec
and rake test:unit
will work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yast-rake is also where we declare --format documentation
in a common place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case I will change the name of the file from spec to test etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm.. ok. I created a symlink instead, your solution was much simpler. About the "--format documentation" should I move it to the rake file instead?
ENV["LANG"] = "en_US.UTF-8" | ||
|
||
# load it early, so other stuffs are not ignored | ||
if ENV["COVERAGE"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay!
If we also add /coverage/
to top level .gitignore
, the CI check should go green again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
On the subject of single or double quotes around strings: double quotes win. We have a YaST Ruby style, https://github.com/yast/yast-devtools/blob/4dcb03ac7e4816a4f858534163393767d52df5fb/ytools/y2tool/rubocop-0.71.0_yast_style.yml#L35 , which is enforced by RuboCop in the "major" repositories. We deviate in some points from the "community ruby style", double quotes being the most visible point. |
51872bc
to
695bc3b
Compare
I think it looks good now. So lets merge it and solve other improvements as another pull requests. |
✔️ Public Jenkins job #11 successfully finished |
✔️ Internal Jenkins job #5 successfully finished |
Currently the package doesn't have any unit test.
For motivation, please see : https://progress.opensuse.org/issues/69394
for easier code understanding, here are some helpful links:
builtins https://github.com/yast/yast-ruby-bindings/blob/master/src/ruby/yast/builtins.rb
rspec for builtins, examples with Builtins.add https://github.com/yast/yast-ruby-bindings/blob/master/tests/builtins_spec.rb
ops https://github.com/yast/yast-ruby-bindings/blob/master/src/ruby/yast/ops.rb
deep_copy https://github.com/yast/yast-ruby-bindings/blob/master/src/ruby/yast/yast.rb