Improve expand_path warnings

[deivid-rodriguez]

Summary

My changes improve the warnings printed when passing an absolute path to aruba's expand_path, and also suppress several warnings originated from aruba's own helpers, not from end user's code.

Details

This PR improves warnings like this:

2020-01-01 18:13:47 +0100 WARN: Using absolute paths in Aruba is not recommended. Set config.allow_absolute_paths = true to silence this warning

to instead read as

2020-01-01 19:37:06 +0100 WARN: Aruba's `expand_path` method was called with an absolute path at /home/deivid/Code/aruba/lib/aruba/api/filesystem.rb:157, which is not recommended. Change the call to pass a relative path or set config.allow_absolute_paths = true to silence this warning

The new warning message is better because it gives information about:

• The method being called that it's printing the warning.
• Where the method that it's printing the warning is being called.
• What is the preferred alternative to fix the warning.

In addition, this method prevents several aruba helpers from calling expand_path with absolute paths, and thus removes several warnings that would be printed to end users otherwise, without really being in control of fixing them (other than by creating a PR similar to this one :)).

Motivation and Context

These changes were motivated because I tried to upgrade to latest aruba 1.0.0 pre-release to be able to use #675 in simplecov's specs, and I started seeing several of these warnings. I had no clue on how I should proceed to fix them so I investigated.

I hope this will help other users by either removing "false positive warnings" or by giving better warning messages when they are actually "true positives".

How Has This Been Tested?

I added several tests, and also made sure that warnings are no longer printed during simplecov specs after these changes.

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactoring (cleanup of codebase without changing any existing functionality)
• Update documentation

Checklist:

• I've added tests for my code
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

[olleolleolle]

I find this more-context-offering wording helpful and an improvement.

[mvz]

I'm not sure about this change, since to my mind both create_directory and touch should warn when passed absolute paths. Can you point me to a pull request or something for simplecov where I can take a closer look?

I do agree that the warning messages should be improved so that it is clearer where in the user code the problem lies.

[deivid-rodriguez]

@mvz Since you agree with improving the messages, I removed the change fixing what I think are false positive warnings. Let's discuss the improvements in the warning message first, and then I'll open a separate PR to discuss whether all file helpers should warn or not when passed absolute paths. Sounds good?

[mvz]

Looks good, thanks!

[mvz]

And thanks for trying out the 1.0.0 pre-release, @deivid-rodriguez!

[deivid-rodriguez]

No problem!

I investigated a bit more the warnings I'm getting and I tracked them down to using the The following files should exist: step with absolute paths, which ends up calling the be_an_existing_file matcher with the same absolute paths, which ends up calling expand_path with the same absolute paths.

This seems currently hard to track down (even with the improvements in this PR) because it's not the end user code that calls expand_path but internal aruba's helpers, steps or matchers. For example, the warnings I get look like this:

$ bundle exec cucumber
Using the default profile...
............................2020-01-02 11:53:16 +0100 WARN: Using absolute paths in aruba at /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/aruba-1.0.0.pre.alpha.5/lib/aruba/api/filesystem.rb:38. Using absolute paths in Aruba is not recommended. Set config.allow_absolute_paths = true to silence this warning
2020-01-02 11:53:16 +0100 WARN: Using absolute paths in aruba at /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/aruba-1.0.0.pre.alpha.5/lib/aruba/api/filesystem.rb:30. Using absolute paths in Aruba is not recommended. Set config.allow_absolute_paths = true to silence this warning
2020-01-02 11:53:16 +0100 WARN: Using absolute paths in aruba at /home/deivid/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/aruba-1.0.0.pre.alpha.5/lib/aruba/api/filesystem.rb:30. Using absolute paths in Aruba is not recommended. Set config.allow_absolute_paths = true to silence this warning
..................................................................................................................................................................................................................................................................

55 scenarios (55 passed)
286 steps (286 passed)
0m56.172s

Better than before, because it gives me a place to start looking, but still not very user friendly.

If you really want to fix this for end users, I think that would involve showing proper warnings with proper user code locations for every end user file method, step or matcher being called with an absolute path argument, and then silencing any other warnings that might be triggered internally during its execution.

But, I would honestly remove the warning. The original issue where the problem being fixed here was reported is #478 (not sure if you got other related problems reported over time). The scenario mentioned in the ticket is exactly the same scenario in simplecov that it's throwing the warning for me now. In this case, I consider our scenario perfectly fine, so my solution (now that I know what's happening) will be the config.allow_absolute_paths thing.

But still, if the only problem you've been reported is considered as a false positive, it seems plausible to think that many other users will consider their cases as false positives too, so it seems questionable to warn everybody's code in exchange for protecting from potential bugs. But might be missing context, of course.

Hope this helps!

[deivid-rodriguez]

Also, if you go the "warn everything" way, the expand_path documentation should probably be updated, since it currently shows an example with an absolute path:

aruba/lib/aruba/api/core.rb

Lines 129 to 133 in c2fed66

	# @example Absolute directory
	#
	# # => /foo/bar
	# expand_path('/foo/bar')
	#

without mentioning that it warns.

[mvz]

I opened #691 to improve the message further.