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

Update more tests to use dsc #1480

Merged
merged 4 commits into from
Sep 30, 2024
Merged

Update more tests to use dsc #1480

merged 4 commits into from
Sep 30, 2024

Conversation

leftwo
Copy link
Contributor

@leftwo leftwo commented Sep 27, 2024

This is plucking out non BlockIO parts from: #1474

Give crutest the list of targets it will be operating on so some tests that need that list of targets can still get it.

Updated more tests to use dsc to determine targets instead of having to provide them on the command line.

Updated most of test_up.sh to use dsc for targets instead of supplying them on the command line.
There is still some work required to get hammer tests to use dsc, if we decide to do that.

Give crutest the list of targets it will be operating on so some tests
that need that list of targets can still get it.

Updated more tests to use dsc to determine targets instead of having
to provide them on the command line.

Updated most of test_up.sh to use dsc for targets instead of supplying
them on the command line.
@leftwo leftwo requested a review from mkeeter September 27, 2024 19:51
@leftwo leftwo marked this pull request as ready for review September 27, 2024 19:51
let dsc_client = Client::new(&dsc_str);
Workload::ReplaceBeforeActive { replacement } => {
let dsc_client = match opt.dsc {
Some(dsc_addr) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a small behavior change: previously, there was a default value of http://127.0.0.1:9998; now, we return an error. Dunno if the change is intentional or not!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, before it assumed that any dsc it finds at the default would be correct, but that
may not be what the user wanted. More so now that there is a global dsc option which
should be what these tests use. Hopefully fewer surprises as this makes the dsc server
target required.

} => {
let dsc_client = Client::new(&dsc_str);
Workload::ReplaceReconcile { replacement } => {
let dsc_client = match opt.dsc {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here (there was previously a default value for dsc_str)

@leftwo leftwo merged commit eed6e87 into main Sep 30, 2024
18 checks passed
@leftwo leftwo deleted the alan/tests-use-dsc branch September 30, 2024 16:21
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