-
Notifications
You must be signed in to change notification settings - Fork 374
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
Add more integration tests #611
Add more integration tests #611
Conversation
fdf23a2
to
37b77e3
Compare
|
||
|
||
@pytest.mark.slow | ||
#@pytest.mark.client_args("-t 60") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this deselected, can it be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entire testcase needs to be removed due to #607
tests/integration/conftest.py
Outdated
@@ -46,6 +48,13 @@ def waitfortext(self,txt): | |||
return False | |||
return True | |||
|
|||
def waitfortime(self, timedelay): | |||
'''Wait for time and return output since last command''' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single quotes slipped in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will fix
text = lwm2mserver.waitfortime(20) # wait for max "pmax" | ||
# Pass-Criteria A | ||
assert text.count("Notify from client #0 /3/0/7") > 0 | ||
assert text.count("Notify from client #0 /3/0/8") > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be feasible to check here also the values of Power Source Voltage (7) and Power Source Current (8)? Would be nice to check if the notification messages actually contain the observed values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add payload check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also add upper bound for check for number of notifications (during 20s)
# one update is sent immediately, expect implementation | ||
# to send update at lifetime/2 intervals | ||
# this does not seem to be the case for 120s (more frequent) | ||
assert text.count("Client #0 updated.") >= 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not test for exactly two updates during the lifetime? This however interferes with #613 when testing against 120s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TC removed. (The exact number of updates is implementation dependent)
37b77e3
to
76864e6
Compare
76864e6
to
cc66510
Compare
Add int-102, int-105, int-301, int-302
cc66510
to
c476927
Compare
Add int-102, int-105, int-107, int-301, int-302