Skip to content
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

Extend stub_command to functions #92

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

eviweb
Copy link

@eviweb eviweb commented Apr 27, 2016

Hi,

here is a little enhancement that provides a way to stub the shpec assert function.
It is useful to check its behaviour when it is nested in a function body, like in a matcher for example.

Best regards

Eric

eviweb added 2 commits April 27, 2016 11:08
it allows to stub the assert function and get an expected result
it is useful to test assert result when used in a function body (eq.
in a matcher body)

Signed-off-by: Eric Villard <[email protected]>
@rylnd
Copy link
Owner

rylnd commented May 3, 2016

Perhaps this is more useful as added functionality to stub_command, in cases where the command in question is an existing function that would be lost if we stubbed it. They feel like very similar use cases, and I'd prefer a uniform interface if possible.

While it would still be possible to override shpec functions with this, I struggle to find a reason why you'd want to do so.

eviweb added 3 commits May 3, 2016 20:38
@eviweb
Copy link
Author

eviweb commented May 3, 2016

@rylnd, you're right and it's done.
Stubbing shpec assert is interesting when writing a complex matcher calling assert from within its body.
Then it's possible to check the arguments effectively passed to assert and compare the result to what is expected.

best regards

@eviweb
Copy link
Author

eviweb commented Jun 23, 2016

@rylnd,
I've just noticed that the last build of this PR is marked as passed under travis while it should not be.
I will investigate and hope to provide a patch soon.

Eric

@rylnd
Copy link
Owner

rylnd commented Apr 3, 2017

@eviweb what is the status of this PR?

@eviweb
Copy link
Author

eviweb commented Apr 4, 2017

hi @rylnd,
still at the same point, I'm sincerely sorry but I'm too busy for now.
Eric

@eviweb
Copy link
Author

eviweb commented Apr 4, 2017

@rylnd,
I don't find any way to retrieve the function definition under Dash.
The type command does not provide the same information under Dash as it does under the other supported shells.
Do you have any idea on how to fix this ?

Copy link
Owner

@rylnd rylnd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eviweb thanks again for your work; sorry for taking so long to get back to this.

@@ -36,11 +36,30 @@ end_describe() {
# any identifier as a function name.

stub_command() {
local func_decl="$(get_function_declaration "$1")"
[ -n "${func_decl}" ] && eval "_shpec____${func_decl}"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we ever unset this internal definition?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local vars do not need to be unset as they are local but KSH does not support the local keyword, I corrected that in another pending branch...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not referring to func_decl, I'm referring to the function that is evald here. After calling stub_command foo; unstub_command foo there will be still be a _shpec___foo function in the namespace, correct?

get_function_declaration()
{
if [ "${SHELL}" = "dash" ]; then
type "$1" | tail -n +2
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be misunderstanding this: I thought that everything except for dash can use type; this seems to indicate the opposite.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the discussion in #89, dash's own docs, and our previous conversation, I'm fine keeping use of the (non-POSIX) type and typeset commands. If this becomes a problem, we can revisit (or move this feature to a plugin).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I explained in my last comment #92 (comment), Unfortunately there's currently no way I see to retrieve the body of a function under Dash 😞

@@ -86,6 +86,15 @@ line'
assert equal "$(curl)" "stubbed body"
unstub_command "curl"
end

it "prevents loosing stubbed function"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/loosing/losing please!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be done if we can find a solution for the problem mentioned here #92 (comment)

@eviweb
Copy link
Author

eviweb commented Apr 26, 2017

@rylnd,
in fact the title of this PR does no more represent really what it now tries to address.
I started to provide a way to stub the assert function, but I noticed that stubbed functions (not commands) are lost after having been unstubbed.
Putting aside the issue of the type command under Dash, I should be able to solve this problem under the other supported shells.
Maybe should it effectively be done as a plugin as suggested, instead of being part of the core for now.
What do you think about this?
In that case, do you want to integrate plugins in this repo?
Should plugins be considered as custom matchers or do you plan to provide a new entrypoint for plugins as you have done for custom matchers? Or is there another preferable way to go?
Best
Eric

@rylnd
Copy link
Owner

rylnd commented Apr 26, 2017

@eviweb my mistake; I thought I had seen typeset work under dash, but I was mistaken.

@hlangeveld you had done some research for #89 into how to obtain a command's type with various POSIX(ish) builtins; are you aware of any way to obtain the function declaration in dash?

If not, then we either drop dash support or we move this work to a plugin. I will give #108 some thought; I would love to hear others' thoughts/ideas there as well.

@rylnd rylnd changed the title Stub assert Extend stub_command to functions Apr 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants