Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: remove direct uses of *TestCluster #107986

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

knz
Copy link
Contributor

@knz knz commented Aug 1, 2023

All commits except the last are from #107866 and prior.

Epic: CRDB-18499

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz knz force-pushed the 20230801-testcluster branch from fccefe9 to 6c4fb4e Compare August 2, 2023 01:54
We should really only look at top level servers through an interface.

Release note: None
@knz knz force-pushed the 20230801-testcluster branch from 6c4fb4e to cf66a7d Compare August 2, 2023 16:03
TestCluster has an interface. Use it.

Release note: None
@knz knz force-pushed the 20230801-testcluster branch from cf66a7d to ebd283c Compare August 2, 2023 16:21
craig bot pushed a commit that referenced this pull request Aug 10, 2023
107302: storage: add method to ingest external files, rename IngestExternalFiles r=RaduBerinde a=itsbilal

Requires cockroachdb/pebble#2753

This change renames the existing IngestExternalFiles method on storage.Engine to IngestLocalFiles, and adds a new IngestExternalFiles that ingests pebble.ExternalFile, for use with online restore.

Depends on cockroachdb/pebble#2753.

Epic: none

Release note: None

108402: serverutils: remove ad-hoc code from StartNewTestCluster r=yuzefovich a=knz

This function is a convenience alias for NewTestCluster+Start. This should not contain custom logic specific to certain tests. Any custom logic should be conditional on testing knobs and put inside `(*testcluster.TestCluster).Start()` instead.

(The code removed here was mistakenly added in the wrong place in 70f85cd).

Release note: None
Needed for #107986.
Epic: CRDB-18499

108446: kv: skip TestConstraintConformanceReportIntegration under deadlock r=erikgrinaker a=nvanbenschoten

Fixes #108430.

This commit avoids flakiness in `TestConstraintConformanceReportIntegration` by skipping the test under deadlock builds. It has been observed to run slowly and flake under stress, and we see the same kinds of behavior under deadlock builds.

Release notes: None

108451: schemachanger: Refactor tests for concurrent schema changer behaviors r=Xiang-Gu a=Xiang-Gu

1. It cleans up some redundant tests about concurrent schema changer behavior and refactor in a new simpler, cleaner test
2. It adds an integration style test for testing concurrent schema change behaviors where we run many schema changes for an extended period of time and assert that all of they eventually succeed and the descriptors end up in the expected state.

Fix #108140
Fix #107223

Epic: None
Release note: None

108492: kv: remove errSavepointInvalidAfterTxnRestart r=knz a=nvanbenschoten

This commit simplifies logic in `checkSavepointLocked`.

Epic: None
Release note: None

108497: sql: don't start default test tenant in MT admin function tests r=yuzefovich a=yuzefovich

These tests themselves start multiple tenants, so there is no need to create a default test tenant (doing that also makes it a bit more confusing because the default tenant as well as the first test tenant share the same TenantID effectively making it two SQL pod config, which is confusing). Starting the default test tenant was enabled recently in c899661 when we enabled the CCL license, and we have seen at least one confusing failure that is possibly related to this.

Starting the default test tenant was originally added in cfa4375, but I don't see a good reason for it.

This PR is opportunistic fix of #108081.

Fixes: #108081.

Release note: None

108502: kvstreamer: add more assertions to RequestsProvider.enqueue r=yuzefovich a=michae2

If we ever enqueue zero-length requests, it could cause a deadlock where the `workerCoordinator` is waiting for more requests and the enqueuer is waiting for results. Add assertions that we never do this.

Informs: #101823
Release note: None

108517: roachtest: pin liveness lease to live node in lease prefs test r=erikgrinaker a=kvoli

The lease preferences roachtest could occasionally fail, if the liveness leaseholder were on a stopped node. We should address this issue, for now, pin the liveness lease to a live node to prevent flakes.

Informs: #108512
Resolves: #108425
Release note: None

Co-authored-by: Bilal Akhtar <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Xiang Gu <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Austen McClernon <[email protected]>
craig bot pushed a commit that referenced this pull request Aug 14, 2023
108628: ui: user cluster settings from redux r=maryliag a=maryliag

Part Of #108373

https://www.loom.com/share/e8b2bc222db848f7a55d442c23c31fe6

Use the value of the cluster setting
`sql.index_recommendation.drop_unused_duration` from redux, instead of adding as part of the select.
With this change, now users with VIEWACTIVITY or
VIEWACTIVITYREDACTED can see index recommendations on the console, without the need the view cluster settings permission.
This commit fixes on api from Database pages (Database Details and Table Details).

Release note: None

108636: tests: rename `StartNewTestCluster` to `StartCluster` r=yuzefovich a=knz

Part of #107986.
Epic: CRDB-18499

This follows for symmetry with `StartServer`.

This is a simple searc-replace substitution:

`serverutils.StartNewTestCluster` -> `serverutils.StartCluster`

Release note: None

108721: sqlccl: remove base.TODOTestTenantDisabled r=yuzefovich a=yuzefovich

In all of the touched tests we control the tenants explicitly. Also add some log scopes.

Informs: #76378.
Epic: CRDB-18499.

Release note: None

108734: ui: fix filter font size r=maryliag a=maryliag

On CC Console, the font size of the filter was
using a wrong value inherited from another class.
This commit makes the value explicit to be consistent.

Before
<img width="623" alt="Screenshot 2023-08-14 at 3 08 24 PM" src="https://github.com/cockroachdb/cockroach/assets/1017486/59bc6306-642e-4fbb-947f-500dd335ec10">


After
<img width="1187" alt="Screenshot 2023-08-14 at 3 07 39 PM" src="https://github.com/cockroachdb/cockroach/assets/1017486/27034065-97a5-40a5-863d-231debc6faad">


Epic: none
Release note: None

Co-authored-by: maryliag <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants