dev: override bazel timeout when run under stress

[irfansharif]

When running under stress and no timeout is specified, we want
to respect the timeout passed down to stress¹. Not doing so has bazel
default to a timeout based on the test target's size², which is not
what we want.

Release note: None

Footnotes

1. Through --stress-arg=-maxtime or if nothing is specified, a
   -maxtime of 0 that's taken as "run forever") ↩

2. https://docs.bazel.build/versions/main/test-encyclopedia.html#role-of-the-test-runner ↩

[cockroach-teamcity]

This change is 

[irfansharif]

Aside: I see we ripped out the automatic recording stuff in #68514, but the datadriven tests here still consults these files. I had to edit them by hand, which I stumbled with expecting --rewrite to pick it up like we do elsewhere (and the fact that two files needed updating -- definitely worth adding in the README here).

In the future we may consider migrating to a proper mocking framework for dev.c

I'm curious what we're imagining a proper mocking framework for dev to look like? And what would it give us over using recordings in the way we were before?

[rickystewart]

Aside: I see we ripped out the automatic recording stuff in #68514, but the datadriven tests here still consults these files.

Yeah, the recording wasn't useful. It introduced so many diffs that heavy manual editing of the testdata was still necessary, and at that point it's way less time-consuming to just manually make the changes you were expecting. This also scaled poorly as more tests were added.

I'm curious what we're imagining a proper mocking framework for dev to look like? And what would it give us over using recordings in the way we were before?

For example, a limitation of the current design is that the recording framework doesn't allow us to capture that certain events should fail, which means we can't test edge/failure cases with the datadriven tests.

[irfansharif]

It introduced so many diffs that heavy manual editing of the testdata was still necessary

That sounds surprising and was probably a bug if anything. The intent was that -record -from-checkout=<path> -rewrite with obviate all need for any manual editing. Do you remember what you were doing that made these edits necessary? I'd love to learn.

we can't test edge/failure cases with the datadriven tests.

We use datadriven tests to test failure cases all throughout our logic tests, etc, and I think it's particularly well suited for it too. Here are some examples of us doing just that:

cockroach/pkg/cli/testdata/logflags

Lines 363 to 368 in 31e7416

	# Logging to stderr without stderr capture causes an error in the default config.
	run
	start
	--log=capture-stray-errors: {enable: false}, sinks: {stderr: {filter: INFO}}
	----
	error: yaml: did not find expected key

cockroach/pkg/spanconfig/spanconfigkvaccessor/testdata/partial_overlap

Lines 20 to 23 in 31e7416

	kvaccessor-update
	upsert [d,e):Y
	----
	err: expected to find single row containing upserted spans

It's ends up being a matter of writing the error output in way that's distinguishable from the happy path, which the recorder didn't do, but I think it's a minor change to make.

[irfansharif]

bors r+

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build succeeded:

• GitHub CI (Cockroach)