Skip to content

jobsprotectedts: fixes test failure related to error wrapping#72384

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
adityamaru:fix-72371
Nov 3, 2021
Merged

jobsprotectedts: fixes test failure related to error wrapping#72384
craig[bot] merged 1 commit intocockroachdb:masterfrom
adityamaru:fix-72371

Conversation

@adityamaru
Copy link
Copy Markdown
Contributor

Fixes: #72371

Release note: None

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

ensureNotExists := func(ctx context.Context, txn *kv.Txn, ptsID uuid.UUID) (err error) {
_, err = ptp.GetRecord(ctx, txn, ptsID)
if err == nil {
return errors.New("found pts record, waiting for ErrNotExists")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if these should be assertion errors instead? I'm leaning towards no based on my understanding of assertion errors being used when we think a situation is a programmatic error, or violation of an invariant.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agreed, this error is expected.

ensureNotExists := func(ctx context.Context, txn *kv.Txn, ptsID uuid.UUID) (err error) {
_, err = ptp.GetRecord(ctx, txn, ptsID)
if err == nil {
return errors.New("found pts record, waiting for ErrNotExists")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agreed, this error is expected.

@adityamaru
Copy link
Copy Markdown
Contributor Author

bors r=stevendanna

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 3, 2021

Build succeeded:

@craig craig bot merged commit f339331 into cockroachdb:master Nov 3, 2021
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.

jobs/jobsprotectedts: TestSchedulesProtectedTimestamp failed

3 participants