[WIP] Operating System compatibility of tools and actions

[janpio]

Checklist

• I've run bundle exec rspec from the root directory to see all new and existing tests pass
• I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
• I've read the Contribution Guidelines
• I've updated the documentation if necessary.

Motivation and Context

Description

closes #11859

[fastlane-bot-helper]

	1 Warning
⚠️	Pull Request is Work in Progress

Generated by 🚫 Danger

[janpio]

Open questions:

1. How to properly test this?
2. How to stop code execution if is_compatible returns false? (output, exception, crash, return false?)
   (See https://github.com/fastlane/fastlane/pull/11893/files#diff-394215798b66f7b366c9d2d7ecee29e2R316)
3. How to motivate developers to implement is_compatible in their actions? (is_supported uses an UI.crash!)
   (See https://github.com/fastlane/fastlane/pull/11893/files#diff-1096193739fd431d5d8b5b9dd7a8edf4R154)
4. How to not break other, existing tests that use something like expect(UI).to receive(:message).with(...) with the messages this outputs on uncompatible or unkown-compatible operating systems?
   (See e.g. https://circleci.com/gh/fastlane/fastlane/15740?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link)

Open TODOs:

5. Implement default is_compatible in plugin action template.
   (See https://github.com/fastlane/fastlane/pull/11893/files#diff-f19b031097b9ddb662d5df9023279128R47)
6. Check generated action documentation.

[revolter]

1. How to properly test this?

Do we need to have unit tests for this or wait for people to use the respective actions on a Linux machine?
Having a unit test passing on Linux for an action means that we can mark it with true (regarding its compatibility)?

2. How to stop code execution if is_compatible returns false? (output, exception, crash, return false?)
   (See [WIP] Operating System compatibility of tools and actions  #11893/files#diff-394215798b66f7b366c9d2d7ecee29e2R316)

I think that this call:

fastlane/fastlane/lib/fastlane/runner.rb

Line 315 in 7792837

UI.error("Action '#{name}' is not compatible to run on operating system '#{operating_system}'.")

could be replaced with a call to user_error!, that, IIRC, automatically stops the executing lane.

3. How to motivate developers to implement is_compatible in their actions? (is_supported uses an UI.crash!)
   (See [WIP] Operating System compatibility of tools and actions  #11893/files#diff-1096193739fd431d5d8b5b9dd7a8edf4R154)

I'd say that using crash! is a correct approach, but don't take my word for it.

4. How to not break other, existing tests that use something like expect(UI).to receive(:message).with(...) with the messages this outputs on uncompatible or unkown-compatible operating systems?
   (See e.g. circleci.com/gh/fastlane/fastlane/15740?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link)

I think there is a way in the fastlane codebase to disable some output during tests, but don't know where I saw it.

[janpio]

1. How to properly test this?

Do we need to have unit tests for this

Definitely - but my question referred to the code I wrote for is_compatible to work. I want to make sure that this thing is super stable as it will be executed before each and every action is executed.

... or wait for people to use the respective actions on a Linux machine?
Having a unit test passing on Linux for an action means that we can mark it with true?

But those are good questions as well. In general we assume that the fastlane codebase is fully covered with tests, and as almost all tests are passing on Linux and Windows we could assume that all will be good.

But I don't fully trust that yet, which is why I prefer marking both new platforms (Linux, Windows) as nil meaning "this will probably work, but we don't know for sure" yet and only change this if it is manually set to true in the action.

Question then is what is enough to set an action to true. Unit tests as you suggest? Actual usage of all parameters? I don't know yet. Suggestions?

2. How to stop code execution if is_compatible returns false? (output, exception, crash, return false?)

I think that this call [...] could be replaced with a call to user_error, that, IIRC, automatically stops the executing lane.

That's a good idea. From fastlane_core README:

fastlane "crash" because of a user error everything that is caused by the user and is not unexpected

Here is the actual implementation:

fastlane/fastlane_core/lib/fastlane_core/ui/interface.rb

Lines 124 to 134 in 41cc295

	# Use this method to exit the program because of an user error
	# e.g. app doesn't exist on the given Developer Account
	# or invalid user credentials
	# or scan tests fail
	# This will show the error message, but doesn't show the full
	# stack trace
	# Basically this should be used when you actively catch the error
	# and want to show a nice error message to the user
	def user_error!(error_message, options = {})
	raise FastlaneError.new(options), error_message.to_s
	end

This might be a nice alternative, that doesn't blame the user for the problem (and probably also doesn't pollute analytics):

fastlane/fastlane_core/lib/fastlane_core/ui/interface.rb

Lines 167 to 177 in 41cc295

	# Use this method to exit the program because of terminal state
	# that is neither the fault of fastlane, nor a problem with the
	# user's input. Using this method instead of user_error! will
	# avoid tracking this outcome as a fastlane failure.
	#
	# e.g. tests ran successfully, but no screenshots were found
	#
	# This will show the message, but hide the full stack trace.
	def abort_with_message!(message)
	raise FastlaneCommonException.new, message
	end

3. How to motivate developers to implement is_compatible in their actions? (is_supported uses an UI.crash!)

I'd say that using crash! is a correct approach, but don't take my word for it.

Yeah, that is to harsh in this case. Actions need an is_supported. But is_compatible has a default setting, so it also works without an implementation. It just would be nice if developers would test their actions on all platforms and make that compatibility explicit by implementing the method and setting the hash.

[revolter]

I'd argue that calling an action on Linux that doesn't support this platform is a user error. It's something that the user is doing while he/she isn't supposed to do so. That's why we also fail early instead of letting it crash.
Also, maybe we do want to know how many people are (or maybe will be) calling actions on unsupported platforms.

[janpio]

Also, maybe we do want to know how many people are (or maybe will be) calling actions on unsupported platforms.

Good point, will keep that in mind and do some research what is possible there.

[janpio]

• user_error! does indeed stop the execution, which can be looked at with the first failing test: https://circleci.com/gh/fastlane/fastlane/16028
• Still looking for a better way to handle additional UI.message output depending on platform in tests that check for specific stuff to be returned there :/

[joshdholtz]

We could probably increase plugin rating if this is defined ^

[janpio]

That's a great idea.

[joshdholtz]

Is it possible to do this without calling super? I don't think we use super anywhere else so not sure if we want to try and make things consistent when people try to implement them? But I do also like this approach too 🤔

Is it possible to also just do something like...

def self.is_compatible?(operating_system)
  [:macos, :windows].include?(operating_system)
end

[janpio]

Is it possible to do this without calling super?

I don't think so as I wanted it to have a "default compatibility" that is present for all actions by default - which would currently be all of them.

(The is_supported? which we already have and is a bit similar doesn't need that as it was required from the beginning and calling an action without it actually stops execution with an error message)

I actually like this better as this way each action doesn't "replicate" the logic to check - but of course this is only important here as the checking logic is a bit more complicated with the defaults.

Is it possible to also just do something like...
[:macos, :windows].include?(operating_system)

I think using just an array only works for yes/no situations. But be have yes/no/unknown to express that it 1) works, 2) doesn't work or 3) hasn't been explicitly set/tested.

[janpio]

After not actively thinking about this for half a year but testing fastlane on Windows and Ubuntu, I think it might make more sense to simplify this to only add a "setting" to the actions that definitely do not work on a platform. No default_compatibility and just an optional is_not_compatible list of platforms where an action definitely does not work (yet).

[janpio]

This will not be implemented.