Add test with exported function execution #34

pawamoy · 2017-10-26T12:43:31Z

It covers the issue in sstephenson/bats#225.

The test is skipped because it only passes on Bash 4.4. I don't know if this is something that can be solved in Bats itself, but at least we have a test for it.

mbland

Just one question for now, will try to take a deeper look this weekend.

mbland · 2017-10-26T12:59:08Z

test/fixtures/bats/exported_function.bats

@@ -0,0 +1,10 @@
+# see issue https://github.com/sstephenson/bats/issues/225
+
+if exported_function; then


What happens if you try a=$(declare -F exported_function >/dev/null) and run it with different Bash versions?

It works with the following:

# in exported_function.bats a=$(declare -F exported_function)

# in bats.bats exported_function() { :; }

I downgraded my Bash to 4.3.48(2), and it gives me this:

$ exported_function() { :; } $ export -f exported_function $ bin/bats --tap test/fixtures/bats/exported_function.bats 1..1 not ok 1 failing test # (in file , line 9, # from function `bats_perform_test' in file libexec/bats-exec-test, line 313, # from function `main' in test file , line 363)

Trying different approaches (original one with if exported_function; then only works in 4.4).

This one works in all versions:

# in exported_function.bats a=$(exported_function && echo 'exported_function')

# in bats.bats exported_function() { true; }

This one works only in 4.4:

# in exported_function.bats exported_function && a='exported_function'

# in bats.bats exported_function() { true; }

This one works in all versions:

# in exported_function.bats a=$(exported_function)

# in bats.bats exported_function() { echo exported_function; }

The last one works only in 4.4:

# in exported_function.bats exported_function && a='exported_function'

# in bats.bats exported_function() { return 0; }

To me it seems that executing exported_function in the current process breaks things up, whereas in a subprocess (like $(exported_function)) works properly.

Interesting, especially when you consider that run bats should be launching a subshell anyways.

Out of curiosity, what happens if you put this if condition in setup() or inside a @test case?

If I put it in setup() { ... } at the beginning of exported_function.bats, it works only in Bash 4.4. If I put it inside the failing test test case, same thing, it works only in Bash 4.4.

mbland · 2017-10-26T13:02:23Z

Also wondering what chapter and verse from https://tiswww.case.edu/php/chet/bash/CHANGES may apply. Maybe:

This document details the changes between this version, bash-4.4-alpha, and
the previous version, bash-4.3-release.

1.  Changes to Bash
ss. Changed the way bash encodes exported functions for inclusion in the
    environment to avoid name collisions with valid variable names and to
    indicate that they are exported functions.

mbland · 2017-10-30T13:16:50Z

Sorry I didn't get to this this weekend; got slammed with another project. Will try to take a look later today.

pawamoy · 2017-11-02T18:32:26Z

I updated the bats test to see what the actual output is when running the failing exported_function test:

not ok 44 execute exported function without breaking failing test output
# (in test file test/bats.bats, line 378)
#   `false' failed
# 
# Output was:
# 1..1
# not ok 1 failing test
# # (in file , line 8,
# #  from function `bats_perform_test' in file libexec/bats-exec-test, line 313,
# #  from function `main' in test file libexec/bats-exec-test, line 363)

pawamoy · 2017-11-02T18:46:27Z

I added the files needed to run the tests against multiple bash versions, but of course you would not merge this. I don't know what the process is, if I delete this last commit when ready to merge or if I send a new, clean PR.

@pawamoy

Closes #34. Incorporates the test code from that PR. This fixes a problem whereby invoking an exported function within a test script would result in an incorrect stack trace. It does this by using `BATS_TEST_SOURCE` to capture a function's frame when its corresponding `BASH_SOURCE` entry is empty. Note: I believe the earlier problem described in sstephenson/bats#225 whereby no output was generated at all was fixed by commit 0f6dde5. My investigation revealed that in Bash versions all the way up through 4.3.48, before this code change, the test failed as @pawamoy reported, with the underlying test fixture producing the following output: not ok 1 failing test (in file , line 7, from function `bats_perform_test' in file libexec/bats-exec-test, line 339, from function `main' in test file , line 391) Starting with Bash 4.4, the test passed. The apparent reason for the failure is that the `BASH_SOURCE` entry corresponding to the test file was getting wiped out when the exported function was called. (Once I converged on this as a potential culprit, I verified it by printing the contents of `BASH_SOURCE` before and after the exported function call.) Since `bats_capture_stack_trace` depends on matching a `BASH_SOURCE` value against `BATS_TEST_SOURCE`, when the `BASH_SOURCE` corresponding to the test function was empty, it would keep collecting stack frames, resulting in the output above. As best I can currently tell, the change accounting for the fix in Bash 4.4 is (from the Bash 4.4 ChangeLog): 1/2/2014 -------- ...snip... 2/26 ---- [bash-4.3 released] ...snip... 11/28 ----- execute_cmd.c - struct func_array_state: new state to save state of BASH_LINENO, BASH_SOURCE, and FUNCNAME during function execution so it can be restored on a jump to top level - restore_funcarray_state: new function to restore func_array_state - execute_function: fill in func_array_state variable, add unwind- protect to restore it on jump to top level, call explicitly at end of function if subshell != 0 (may not be necessary, but safe for now). Fixes bug with local assignments to FUNCNAME reported by Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> 1/1/2015 -------- ...snip... 7/7 --- ...snip... [bash-4.4-alpha frozen]

Emit correct trace after calling exported function

Add test with exported function execution

677222a

mbland requested review from mbland and a team October 26, 2017 12:49

mbland reviewed Oct 26, 2017

View reviewed changes

Add run.sh script and precompiled bash versions

4371e6c

ghost assigned mbland May 30, 2018

ghost added the in progress label May 30, 2018

mbland mentioned this pull request May 30, 2018

Emit correct trace after calling exported function #87

Merged

2 tasks

ghost added review and removed in progress labels May 30, 2018

mbland closed this in #87 Jun 3, 2018

ghost removed the review label Jun 3, 2018

mbland referenced this pull request Jun 3, 2018

Merge pull request #87 from bats-core/exported-function-fix-#34

f837f3d

Emit correct trace after calling exported function

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add test with exported function execution #34

Add test with exported function execution #34

Uh oh!

pawamoy commented Oct 26, 2017

Uh oh!

mbland left a comment

Uh oh!

mbland Oct 26, 2017

Uh oh!

pawamoy Oct 26, 2017

Uh oh!

mbland Oct 31, 2017

Uh oh!

pawamoy Nov 2, 2017

Uh oh!

mbland commented Oct 26, 2017

Uh oh!

mbland commented Oct 30, 2017

Uh oh!

pawamoy commented Nov 2, 2017

Uh oh!

pawamoy commented Nov 2, 2017

Uh oh!

Uh oh!

		@@ -0,0 +1,10 @@
		# see issue https://github.com/sstephenson/bats/issues/225

		if exported_function; then

Add test with exported function execution #34

Add test with exported function execution #34

Uh oh!

Conversation

pawamoy commented Oct 26, 2017

Uh oh!

mbland left a comment

Choose a reason for hiding this comment

Uh oh!

mbland Oct 26, 2017

Choose a reason for hiding this comment

Uh oh!

pawamoy Oct 26, 2017

Choose a reason for hiding this comment

Trying different approaches (original one with if exported_function; then only works in 4.4).

Uh oh!

mbland Oct 31, 2017

Choose a reason for hiding this comment

Uh oh!

pawamoy Nov 2, 2017

Choose a reason for hiding this comment

Uh oh!

mbland commented Oct 26, 2017

Uh oh!

mbland commented Oct 30, 2017

Uh oh!

pawamoy commented Nov 2, 2017

Uh oh!

pawamoy commented Nov 2, 2017

Uh oh!

Uh oh!

Trying different approaches (original one with `if exported_function; then` only works in 4.4).