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

convert still more tests to the sharness framework #104

Merged
merged 5 commits into from
Jan 29, 2024

Conversation

garlick
Copy link
Member

@garlick garlick commented Jan 29, 2024

Here's another batch of conversions.

Problem: test/t16 covers the powerman device status command
using the old test infrastructure.

Convert to sharness.

Remove the old test scripts and data.
Problem: test/t19 and test/t20 cover device timeouts using the
old test infrastructure.

Convert to sharness.

Remove the old test scripts and data.
Problem: test/t25 and test/t25 cover device delays using the
old test infrastructure.

Convert to sharness.

Remove the old test scripts and data.
Copy link
Member

@chu11 chu11 left a comment

Choose a reason for hiding this comment

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

lgtm just few comments/nits

Comment on lines +29 to +33
test_expect_success 'powerman -d works' '
$powerman -h $testaddr -d >device.out &&
echo "test0: state=connected reconnects=000 actions=002 type=vpc hosts=t[0-15]" >device.exp &&
test_cmp device.exp device.out
'
Copy link
Member

Choose a reason for hiding this comment

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

i actually didn't know what -d was and noticed it wasn't in the manpage :-) intentional b/c it's a debugging thing? or error?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like the man page has -D,--device but not -d,--device-all so I guess yeah, an unintentional omission. I'll open a bug.

Comment on lines +16 to +35
test_expect_success 'create test powerman.conf with cycle of only delay' '
cat >powerman.conf <<-EOT
specification "vpcfakecycle" {
timeout 2
plug name { "0" "1" "2" "3" "4" "5" "6" "7" "8"
"9" "10" "11" "12" "13" "14" "15" }
script login {
send "login\n"
expect "[0-9]* OK\n"
expect "[0-9]* vpc> "
}
script cycle {
delay 0.01
}
}
listen "$testaddr"
device "test0" "vpcfakecycle" "$vpcd |&"
node "t[0-15]" "test0"
EOT
'
Copy link
Member

Choose a reason for hiding this comment

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

i'm just curious, is there an old device that didn't output anything when doing a cycle? Just didn't quite understand the purpose of this test compared to the one below.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't too sure what that was about either but I figured it must've been a concern at some point so I'd bring it over.

Comment on lines 150 to 151
wait
wait
Copy link
Member

Choose a reason for hiding this comment

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

wait cut & paste?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we are waiting for two processes, the vpcd and powermand. Ugh, no && though! Will fix.

Problem: test/t48 cover device connection over tcp using the
old test infrastructure.

Convert to sharness.
Incorporate it into the existing t0004-status-query.t since it
follows the same pattern.

Remove the old test scripts and data.
Problem: the list of tests in test/README is out of date.

Remove the tests that have been converted to sharness thus far.
@garlick
Copy link
Member Author

garlick commented Jan 29, 2024

Fixed the missing && and so I'll set MWP - thanks!

@mergify mergify bot merged commit 64adea3 into chaos:master Jan 29, 2024
8 checks passed
@garlick garlick deleted the sharness_conv3 branch January 31, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants