Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Replace IO::CaptureOutput by ours #892

Closed
rurban opened this Issue Dec 20, 2012 · 4 comments

Comments

Projects
None yet
2 participants
Member

rurban commented Dec 20, 2012

Most configure step tests are a mess.
e.g. t/configure/034-step.t tests for the replace_stash option, but passes the test even if the option is not written yet.

genfile output is captured, but the results (the file contents) are never compared.
IO::CaptureOutput is not really needed, it can be simply replaced by an enhanced Parrot::Configure::Utils::capture_output to handle coderefs.

Work is done in the branch rurban/test-capture-gh892

@ghost ghost assigned rurban Dec 20, 2012

@rurban rurban pushed a commit that referenced this issue Dec 20, 2012

Reini Urban [GH #892] Implement our capture and use it
No need to ship a IO::CaptureOutput.
Use the new capture function in t/configure/033-step.t and
t/configure/034-step.t.
t/configure/034-step.t tests now the new replace_stash option correctly. See [GH #891].
3741ae2
Contributor

jkeenan commented Dec 21, 2012

On 12/20/12 3:20 PM, Reini Urban wrote:

Most configure step tests are a mess.
e.g. t/configure/034-step.t tests for the replace_stash option, but passes the test even if the option is not written yet.

genfile output is captured, but the results (the file contents) are never compared.
IO::CaptureOutput is not needed, it can be simply replaced by an enhanced Parrot::Configure::Utils::capture_output to handle coderefs.

Will all instances of IO::CaptureOutput be able to be replaced by
Parrot::Configure::Utils::capture_output()?

Member

rurban commented Dec 24, 2012

jim: Hopefully. But it is very low priority to me. The replacement function is implemented in this branch and looks good to me.

I rather want to fix the step tests first, to be able to check options which should fail and pass.

@rurban rurban pushed a commit that referenced this issue Oct 26, 2014

Reini Urban [GH #892] Implement our own capture, rm lib/IO/CaptureOutput.pm
No need to ship a IO::CaptureOutput.
Use the new capture function in t/configure/033-step.t and
t/configure/034-step.t. t/configure/034-step.t tests now the
new replace_stash option correctly. See [GH #891].
07d8f54

@rurban rurban pushed a commit that referenced this issue Nov 17, 2014

Reini Urban [GH #892] Implement our own capture, rm lib/IO/CaptureOutput.pm
No need to ship a IO::CaptureOutput.
Use the new capture function in t/configure/033-step.t and
t/configure/034-step.t. t/configure/034-step.t tests now the
new replace_stash option correctly. See [GH #891].
5ec2acf

@rurban rurban added this to the 7.0.0 milestone Nov 17, 2014

@rurban rurban pushed a commit that referenced this issue Nov 22, 2014

Reini Urban [GH #892] Implement our own capture, rm lib/IO/CaptureOutput.pm
No need to ship a IO::CaptureOutput.
Use the new capture function in t/configure/033-step.t and
t/configure/034-step.t. t/configure/034-step.t tests now the
new replace_stash option correctly. See [GH #891].
ce4d422

@rurban rurban pushed a commit that referenced this issue Dec 5, 2014

Reini Urban [GH #892] Implement our own capture, rm lib/IO/CaptureOutput.pm
No need to ship a IO::CaptureOutput.
Use the new capture function in t/configure/033-step.t and
t/configure/034-step.t. t/configure/034-step.t tests now the
new replace_stash option correctly. See [GH #891].
c088138

@rurban rurban pushed a commit that referenced this issue Dec 9, 2014

Reini Urban [GH #892] Fixup IO::CaptureOutput removal
Fixes also #1158 windows tests, where I do not have IO::CaptureOutput installed.
bd6f37d
Member

rurban commented Dec 9, 2014

The fixes from Oct 26 were wrong, need some more changes in capture.

@rurban rurban modified the milestones: 6.11.0, 7.0.0 Dec 9, 2014

@rurban rurban pushed a commit that referenced this issue Dec 9, 2014

Reini Urban [GH #892] more capture fixes
actually fill the provided stdout and stderr arguments to capture.
handle missing stderr.

Fixes also the failing win32 tests where I have no IO::CaptureOutput #1158
c1cb400
Member

rurban commented Dec 9, 2014

Tested now okay on linux and windows, with missing IO::CaptureOutput.

@rurban rurban closed this Dec 9, 2014

@rurban rurban pushed a commit that referenced this issue Dec 9, 2014

Reini Urban [codingstd] Variable declared in conditional in capture 0dcf359

@rurban rurban pushed a commit that referenced this issue Dec 9, 2014

Reini Urban [test] one more capture fixup
capture already does the eval, and returns $@ as 4th return value.
GH #892
1cd8b98
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment