Commit Graph

9 Commits

Author SHA1 Message Date
Denis Bilenko 1b2be1b2cb
Add error checking in tests and enable errcheck there (#1980)
## Changes
Fix all errcheck-found issues in tests and test helpers. Mostly this
done by adding require.NoError(t, err), sometimes panic() where t object
is not available).

Initial change is obtained with aider+claude, then manually reviewed and
cleaned up.

## Tests
Existing tests.
2024-12-09 13:56:41 +01:00
Pieter Noordhuis eddaddaf8b
Attempt to reduce test flakiness on Windows (#1845)
## Changes

Test failures indicate that both stdout and stderr are consumed, yet the
content of stdout doesn't end up in the intended output. This can happen
if the goroutines responsible for writing to the combined output buffer
attempt to write to the same underlying buffer concurrently.

Example failure:
```
=== RUN   TestBackgroundCombinedOutput
    background_test.go:65: 
        	Error Trace:	D:/a/cli/cli/libs/process/background_test.go:65
        	Error:      	elements differ
        	            	
        	            	extra elements in list A:
        	            	([]interface {}) (len=1) {
        	            	 (string) (len=1) "2"
        	            	}
        	            	
        	            	
        	            	listA:
        	            	([]string) (len=2) {
        	            	 (string) (len=1) "1",
        	            	 (string) (len=1) "2"
        	            	}
        	            	
        	            	
        	            	listB:
        	            	([]string) (len=1) {
        	            	 (string) (len=1) "1"
        	            	}
        	Test:       	TestBackgroundCombinedOutput
```

With the test body:

ca45e53f42/libs/process/background_test.go (L48-L66)

With the implementation of `WithCombinedOutput`:

ca45e53f42/libs/process/opts.go (L72-L78)

Notice that `c.Stdout` does get the "2", or the test failure would have
included the relevant assertion error. This leads me to believe that
there is a race on writing to `buf` from the two goroutines writing to
`c.Stdout` and `c.Stderr`.

## Tests

The test passes. If this PR has the intended effect remains to be
seen...
2024-10-24 12:03:12 +00:00
Pieter Noordhuis 80136dea5f
Use Go 1.22 to build and test (#1562)
## Changes

This has been released for a while. Blog post:
https://go.dev/blog/go1.22.

## Tests

None besides the unit tests.
2024-07-04 06:54:41 +00:00
Gleb Kanterov 57a5a65f87
Add ApplyPythonMutator (#1430)
## Changes
Add ApplyPythonMutator, which will fork the Python subprocess and
process pipe bundle configuration through it.

It's enabled through `experimental` section, for example:

```yaml
experimental:
  pydabs: 
    enable: true
    venv_path: .venv
```

For now, it's limited to two phases in the mutator pipeline:

- `load`: adds new jobs
- `init`: adds new jobs, or modifies existing ones

It's enforced that no jobs are modified in `load` and not jobs are
deleted in `load/init`, because, otherwise, it will break existing
assumptions.

## Tests
Unit tests
2024-06-20 08:43:08 +00:00
Pieter Noordhuis e3717ba1c4
Fix flaky test in `libs/process` (#1314)
## Changes

The order of stdout and stderr being read into the buffer for combined
output is not deterministic due to scheduling of the underlying
goroutines that consume them. That's why this asserts on the contents
and not the order.
2024-03-26 07:57:48 +00:00
Serge Smertin 42c06267eb
Stub out Python virtual environment installation for `labs` commands (#1057)
This PR removes 15 seconds from `make test` runtime
2023-12-11 16:30:19 +00:00
Andrew Nester ea4153e323
Fixed flaky TestBackgroundCombinedOutputFailure (#978)
## Changes
`TestBackgroundCombinedOutputFailure` was occasionally failing because
combined output could have been in different order, see


https://github.com/databricks/cli/actions/runs/6823883271/job/18558675165?pr=928
2023-11-10 14:09:02 +00:00
Serge Smertin f111b0846e
Added process stubbing for easier testing of launched subprocesses (#963)
## 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
https://github.com/databricks/cli/pull/914 easier

## Tests

`make test`
2023-11-09 14:24:05 +00:00
Serge Smertin 7171874db0
Added `process.Background()` and `process.Forwarded()` (#804)
## Changes
This PR adds higher-level wrappers for calling subprocesses. One of the
steps to get https://github.com/databricks/cli/pull/637 in, as
previously discussed.

The reason to add `process.Forwarded()` is to proxy Python's `input()`
calls from a child process seamlessly. Another use-case is plugging in
`less` as a pager for the list results.

## Tests
`make test`
2023-09-27 09:04:44 +00:00