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 old school power control box tests to sharness #108

Merged
merged 10 commits into from
Jan 29, 2024

Conversation

garlick
Copy link
Member

@garlick garlick commented Jan 29, 2024

This converts the tests for icebox, baytech, cyclades - the devices powerman was originally developed to control.

Also I dropped the gpib testing since the other project it depends on is no longer developed (dev scripts remain).

Finally, the insteon plm test is converted.

Problem: test/t21 and t22 cover baytech rpc3-nc power control
devices using old test infrastructure.

Convert to sharness.

Remove the old test scripts and data.
Problem: test/t23 and t24 cover baytech rpc28-nc power control
devices using old test infrastructure.

Convert to sharness.

Remove the old test scripts and data.
Problem: test/t27 and t28 cover baytech rpc3 power control
devices using old test infrastructure.

Convert to sharness.

Remove the old test scripts and data.
Problem: test/t29, t30, t33, t34 cover LNXI icebox v3, v4
power control using 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 nits i saw

Comment on lines 66 to 73
test_expect_success 'powerman -q shows all off' '
$powerman -h $testaddr -q >query3.out &&
makeoutput "" "t0" "t1" >query3.exp &&
test_cmp query3.exp query3.out
'
Copy link
Member

Choose a reason for hiding this comment

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

not all off, seems t1 is being checked for unknown

echo $! >powermand.pid &&
$powerman --retry-connect=100 --server-host=$testaddr -d
'
test_expect_success 'powerman -q shows all t0 off, t1 unknown' '
Copy link
Member

Choose a reason for hiding this comment

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

perhaps add comment from device file about # N.B.: X10 devices will always show power status "unknown". , t1 always being unknown sorta weird for uniniated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for that reminder - I did not dig deep enough to remember that while converting the test. Will add a note.

@@ -147,8 +147,6 @@ AC_CONFIG_FILES( \
test/sierra.conf \
test/t19.conf \
test/t20.conf \
test/t31.conf \
Copy link
Member

@chu11 chu11 Jan 29, 2024

Choose a reason for hiding this comment

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

commit message error, you mean t31 & t32

Problem: test/t31 and t32 cover LNXI icebox v2 power control
devices using old test infrastructure.

Convert to sharness.

Remove the old test scripts and data.
Problem: powerman includes tests for the hp3488 and ics8064
GPIB switch modules using the gpib-utils project, but that
project is largely abandoned.

Leave the dev scripts in case anyone is using them but drop
the simulator and tests.
Problem: t19 and t20 were converted to sharness but the original
tests were not removed.

Remove old test scripts and data
Problem: test/t46 covers cyclades pm series power control
devices using old test infrastructure.

Convert to sharness.

Remove the old test scripts and data.
Problem: test/t41 covers the insteon power line modem device
using old test infrastructure.

Convert to sharness.

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

Thanks! Pushing fixes and setting MWP.

@mergify mergify bot merged commit f3dc689 into chaos:master Jan 29, 2024
8 checks passed
@garlick
Copy link
Member Author

garlick commented Jan 29, 2024

oops, I pushed changes and set MWP right afterwards, but apparently the earlier passing tests were sufficient for mergify to merge without waiting for CI to finish. That's not good - any idea how to fix that? It doesn't work that way in the flux repos AFAIK

@chu11
Copy link
Member

chu11 commented Jan 29, 2024

Hmmmmm I’m wondering if we need to add a branch protection too. I’ll look into it.

@chu11
Copy link
Member

chu11 commented Jan 29, 2024

dunno if this will fix it, but set these branch protections that these builders gotta complete

image

@garlick
Copy link
Member Author

garlick commented Jan 29, 2024

That seems like it has a high probability of success. Thanks :-)

@garlick garlick deleted the sharness_conv4 branch January 30, 2024 14:49
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