Skip to content

Conversation

@nfx
Copy link
Collaborator

@nfx nfx commented Nov 7, 2023

Changes

This PR makes unit testing with subprocesses fast.

	ctx := context.Background()
	ctx, stub := process.WithStub(ctx)
	stub.WithDefaultOutput("meeee")

	ctx = env.Set(ctx, "FOO", "bar")

	out, err := process.Background(ctx, []string{"/usr/local/bin/meeecho", "1", "--foo", "bar"})
	require.NoError(t, err)
	require.Equal(t, "meeee", out)
	require.Equal(t, 1, stub.Len())
	require.Equal(t, []string{"meeecho 1 --foo bar"}, stub.Commands())

	allEnv := stub.CombinedEnvironment()
	require.Equal(t, "bar", allEnv["FOO"])
	require.Equal(t, "bar", stub.LookupEnv("FOO"))

This should make further iterations of #914 easier

Tests

make test

This PR makes unit testing with subprocesses fast.

```
ctx := context.Background()
	ctx, stub := process.WithStub(ctx)
	stub.WithDefaultOutput("meeee")

	ctx = env.Set(ctx, "FOO", "bar")

	out, err := process.Background(ctx, []string{"/usr/local/bin/meeecho", "1", "--foo", "bar"})
	require.NoError(t, err)
	require.Equal(t, "meeee", out)
	require.Equal(t, 1, stub.Len())
	require.Equal(t, []string{"meeecho 1 --foo bar"}, stub.Commands())

	allEnv := stub.CombinedEnvironment()
	require.Equal(t, "bar", allEnv["FOO"])
	require.Equal(t, "bar", stub.LookupEnv("FOO"))
```
@nfx nfx added the Tech Debt Issues related to technical debt (tests, refactoring and etc.) label Nov 7, 2023
@nfx nfx requested a review from pietern November 7, 2023 21:51
@nfx nfx enabled auto-merge November 7, 2023 23:58
Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

I like the idea, but doesn't look finished yet.

@nfx nfx requested a review from pietern November 8, 2023 16:37
@nfx nfx requested a review from pietern November 9, 2023 11:48
Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

Please update title to something more changelog friendly -- e.g.

Add process stubbing for easier testing

@nfx nfx added this pull request to the merge queue Nov 9, 2023
@pietern pietern removed this pull request from the merge queue due to a manual request Nov 9, 2023
@nfx nfx changed the title Added ctx, stub := process.WithStub(ctx) for speed of DevEx Added process stubbing for easier testing of launched subprocesses Nov 9, 2023
@nfx nfx added this pull request to the merge queue Nov 9, 2023
Merged via the queue into main with commit f111b08 Nov 9, 2023
@nfx nfx deleted the process/stub branch November 9, 2023 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Tech Debt Issues related to technical debt (tests, refactoring and etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants