Differential D19334

redo service menu ruby helpers from ground up more or less
ClosedPublic
Actions

Authored by sitter on Feb 26 2019, 12:48 PM.

Details

Reviewers

elvisangelaccio

Group Reviewers

Dolphin

Commits

R318:644d6f85223d: redo service menu ruby helpers from ground up more or less

Summary

apply ruby community style guidelines
full rewrite fixing, among other things:
- inefficient/unreadable String#end_with reimplementation
- inefficient use of Kernel.system (forked shell to fork a shell)
- inefficient/unreadable Dir.glob reimplementation
- inefficient File initialization for single chmod
- invocation conditions are now actually readable
- invocation conditions now also force +x on argless scripts, not just scripts that need arguments
- repetitive conditions are now expressed as loops on argument arrays
- mime detection now uses xdg-mime instead of file (xdg-mime internally may fall back to mime but will prefer higher level tools such as kmimetypefinder5; giving better results overall)
- return values of "backtick forks" are now checked and will produce suitable errors on stderr
- fail now takes a log_msg argument which is printed to stderr. this is in addition to the error raised as notification for the user, as that is unfortunately not so useful for diagnostics
- overall error handling and logging of problem causes is much improved
add license headers. the original code was actually fairly exhaustive, so this really should have had a header to begin with. the code was originally introduced in svn r1045663 on Nov 6 14:56:35 2009 UTC
add blackbox tests. in the interest of keeping the scripts actually simple scripts (as opposed to a bunch of classes used by even simpler scirpts) they are now also covered by test rigging which runs them as scripts (again, as opposed to individual unit testing of distinct units)
- the tests optionally can use simplecov to gather coverage metrics
- also wired up to ctest so it actually gets run

structurally there is actually a fair amount of overlap between the two
scripts, but again, in the interest of keeping things simple I think it's
better to live with that instead of refactoring a shared library out of
it and then use heavy-duty meta-programming

Diff Detail

Repository

R318 Dolphin

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

sitter created this revision.Feb 26 2019, 12:48 PM

Restricted Application added a project: Dolphin. · View Herald TranscriptFeb 26 2019, 12:48 PM

Restricted Application added a subscriber: kfm-devel. · View Herald Transcript

sitter requested review of this revision.Feb 26 2019, 12:48 PM

Harbormaster completed remote builds in B8866: Diff 52604.Feb 26 2019, 12:48 PM

sitter added a reviewer: Dolphin.Feb 26 2019, 12:48 PM

don't use strip!. it comes back nil when nothing was stripped which is not useful behavior in the way it is used (i.e. if nothing needed stripping we really don't care)

Harbormaster completed remote builds in B8880: Diff 52648.Feb 26 2019, 4:37 PM

I get this error when running the test:

16/16 Test #16: servicemenutest ......................***Failed    0.07 sec
SimpleCov not loaded
/usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require': cannot load such file -- test/unit (LoadError)
        from /usr/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
        from /home/elvis/dev/kde/dolphin/src/settings/services/test/test_helper.rb:78:in `<top (required)>'
        from /home/elvis/dev/kde/dolphin/src/settings/services/test/test_run.rb:22:in `require_relative'
        from /home/elvis/dev/kde/dolphin/src/settings/services/test/test_run.rb:22:in `<main>'

Should we add a ruby dependencies check in cmake? (if that's even possible?)

Could this be a packaging problem in your distro?

test-unit is (meant to be) bundled with ruby itself https://github.com/ruby/ruby/blob/v2_6_0/gems/bundled_gems and I just wiped my 2.6.0 completely and reinstalled and am absolutely sure that by default it works without installing any additional dependencies.

In D19334#428879, @sitter wrote:

Could this be a packaging problem in your distro?

test-unit is (meant to be) bundled with ruby itself https://github.com/ruby/ruby/blob/v2_6_0/gems/bundled_gems and I just wiped my 2.6.0 completely and reinstalled and am absolutely sure that by default it works without installing any additional dependencies.

You are right, archlinux explicitly removes the bundled gems: https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/ruby#n62

I guess dolphin developers who use archlinux should be able to figure it out as I did, so feel free to ship it. Thanks!

This revision is now accepted and ready to land.Mar 17 2019, 2:34 PM

Closed by commit R318:644d6f85223d: redo service menu ruby helpers from ground up more or less (authored by sitter). · Explain WhyMar 18 2019, 1:04 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

		Path
M		src/settings/services/servicemenudeinstallation (95 lines)
M		src/settings/services/servicemenuinstallation (204 lines)
A	M	src/settings/services/test/service_menu_deinstallation_test.rb (121 lines)
A	M	src/settings/services/test/service_menu_installation_test.rb (122 lines)
A	M	src/settings/services/test/test_helper.rb (78 lines)
A	M	src/settings/services/test/test_run.rb (24 lines)
M		src/tests/CMakeLists.txt (2 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	52604	33581c6		Feb 26 2019, 12:48 PM	★	★
Diff 2	52648	33581c6	don't use `strip!`. it comes back nil when nothing was stripped which is not…	Feb 26 2019, 4:37 PM	★	★
Diff 3	54209	363fd1a	R318:644d6f85223d486a412af9aaf4b40ef3d5d6705b	Mar 18 2019, 1:02 PM	★	★