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

testfactory: automatically set FinalizedAt if finalized state #585

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

bgentry
Copy link
Contributor

@bgentry bgentry commented Sep 9, 2024

This was a small quality of life improvement I ran into while working on Pro stuff. I figure that by default, the testfactory package should try to do the right thing and avoid requiring every field to be specified when there are reasonable defaults that could be set based on other values.

In that vein, if the job being created is in a finalized state, automatically set the FinalizedAt timestamp so that callers don't always have to do so. There was only one test that this broke, which was an easy fix.

If the job being created is in a finalized state, automatically set the
`FinalizedAt` timestamp so that callers don't always have to do so.
@bgentry bgentry requested a review from brandur September 9, 2024 02:22
@bgentry
Copy link
Contributor Author

bgentry commented Sep 9, 2024

We may also want to move this into rivershared at some point. I copied it over to unblock stuff last night but we could probably use it in riverui and River pro too.

Merging for now though!

@bgentry bgentry merged commit f147584 into master Sep 9, 2024
14 checks passed
@bgentry bgentry deleted the bg-test-factory-improvement branch September 9, 2024 12:26
tigrato pushed a commit to gravitational/river that referenced this pull request Dec 18, 2024
…ueue#585)

If the job being created is in a finalized state, automatically set the
`FinalizedAt` timestamp so that callers don't always have to do so.
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