Merge into Sle 12 sp1

[teclator]

No description provided.

[jreidinger]

this is problematic as requiring client cause its execution. So we usually avoid it. E.g. all mocking is not done during such run. For auto client I expect that without arguments it do nothing, but you should not rely on it. As e.g. with using https://github.com/yast/yast-yast2/blob/master/library/general/src/lib/installation/auto_client.rb#L60 new base class for auto clients it will raise exception.

[jreidinger]

btw nasty workaround for it is using load File.expand_path("../../src/clients/firewall_auto.rb", __FILE__) instead of calling main.

[teclator]

@jreidinger we will apply the nasty workaround for SP1, as we can change it for SP2.

[jreidinger]

then I really suggest to not use require_relative and instead use WFM.CallFunction https://github.com/yast/yast-ruby-bindings/blob/master/src/ruby/yast/wfm.rb#L164 with given parameters as it is much closer to expected usage. So instead of stubbing args you call expect(Yast::WFM.CallFunction("firewall_auto", ["Summary"])).to eq "Some summary"

[mvidner]

btw nasty workaround for it is using load File.expand_path("../../src/clients/firewall_auto.rb", __FILE__) instead of calling main.

I don't know how that is supposed to help. main is executed anyway.

[mvidner]

So I would drop commit d96edf3 and instead add a comment at the require

the client will be run which is bad in general but this one only complains in the log if func is not specified, so it is OK

[jreidinger]

@mvidner

btw nasty workaround for it is using load File.expand_path("../../src/clients/firewall_auto.rb", FILE) instead of calling main.
I don't know how that is supposed to help. main is executed anyway.

Tricky part is that is it always executed, so you can do mock this and that and then load and in different scenario mock it in other way and call load

[mvidner]

The tricky part is that is it always executed, so you can mock this and that and then load

OK, that could work, but in this case that is not done and I think it is not worth it.

[jreidinger]

@teclator what is status here?

[teclator]

@jreidinger it has not been lgmted yet, in that momment it should merged, @Imobach what do you think about?

[jreidinger]

ok, so LGTM from me. it is always good to have tests :)