| opendevreview | Rodolfo Alonso proposed openstack/devstack master: [OVN] Add support for the Neutron OVN agent service https://review.opendev.org/c/openstack/devstack/+/904171 | 08:14 |
|---|---|---|
| dansmith | gmann: kopecmartin: what's the definition of a negative test for tempest? | 15:10 |
| dansmith | my feeling is that it's something like "tests something that shouldn't be possible" but I'm wondering if it also includes some things like "try to break something and make sure the failure is correct" | 15:12 |
| dansmith | is it just the former or also the latter? | 15:12 |
| kopecmartin | dansmith: the first one - https://opendev.org/openstack/tempest/src/branch/master/HACKING.rst#negative-tests | 15:29 |
| dansmith | ouch, RTFM'd :P | 15:29 |
| kopecmartin | "try to break" sounds very not deterministic, more of a stress test in a sense | 15:30 |
| dansmith | hmm, not sure the docs there really cover my second case | 15:30 |
| dansmith | we have a test that creates an image, tries to snapshot it, and then deletes the image mid-snapshot and makes sure we get a particular error | 15:31 |
| dansmith | I was chasing a traceback in the server side logs and saw it was in the ImagesTest (and not a negative one) so I figured it was something expected to work | 15:32 |
| dansmith | not proposing a change to the location of the test or anything, just kinda rambling about whether my expectations of what goes where is in line | 15:33 |
| kopecmartin | i'm having flashbacks about a similar case, probably long time ago we discussed something like that, or maybe a new test we wanted to write, the outcome was that stopping an operation mid-fly is not 100% reproducible and thus a test like that should not be in tempest | 16:07 |
| dansmith | yeah, I don't know that we have problems with this being flaky, but you're right, it's a race | 16:09 |
| gmann | dansmith: on negative tests, if anything not coverable at project side functional tests or it is good scenario tests then it make sense to add in tempest. but most of negative tests testing API/DB param/limits are not welcome in tempest as they can be easily covered by unit tests at project side | 19:18 |
| dansmith | yeah, I get that for sure (and agree) | 19:19 |
| gmann | your case seems ok but as kopecmartin mentioned if it is 100% operable via API operation (delete image min-snapshot) then we cna add | 19:19 |
| dansmith | to be clear, it's already in the tree | 19:19 |
| gmann | ok | 19:19 |
| dansmith | I was surprised to see it in a non-NegativeTest class | 19:19 |
| dansmith | because it seems like a negative test to me (and maybe not very appropriate in general) | 19:20 |
| gmann | ohk, that might be mistake when we added it. agree that these cases should be in negative test class | 19:20 |
| dansmith | Like, I saw the trace it was creating in n-cpu logs, saw it was for a non-negative test class, and assumed it was something really broken | 19:21 |
| dansmith | but in reality it was expected because it was a snapshot-and-delete-fast scenario in ImagesTest | 19:22 |
| dansmith | there are other such errors in the log, but all for ImagesNegativeTest... | 19:22 |
| gmann | i see, I also get sometime confuse with those failure logs but seeing they are from negative test class help to know | 19:23 |
| gmann | dansmith: which test that was? | 19:27 |
| dansmith | gmann: https://github.com/openstack/tempest/blob/master/tempest/api/compute/images/test_images.py#L52 | 19:27 |
| gmann | dansmith: so in this tests image should get deleted while it in saving state right? | 19:29 |
| gmann | i mean this seems positive test | 19:30 |
| dansmith | okay yeah, I'm coming at it from the other perspective of what I'm actually testing | 19:33 |
| dansmith | but I see, one way to read this is "make sure we can delete an image while snapshotting" which is positive | 19:34 |
| dansmith | since I'm reviewing glance changes around this, I'm thinking about it from the perspective of "make sure the snapshotting gracefully handles a deleted-in-the-middle image" which is more negative | 19:34 |
| dansmith | so, fair enough :) | 19:34 |
| dansmith | and given this is a compute test, I guess the positive reading is the right one in this case :P | 19:35 |
| gmann | ohk, so what snapshot API suppose to return in this case? | 19:35 |
| gmann | 400 ? | 19:35 |
| dansmith | no, snapshot api is async, so it has already returned, but it kicks off a long-running process in glance which we monitor | 19:35 |
| dansmith | but in this case nova is just adding a location (which is the new API being tested) to glance once it's done, but the image has been deleted so that update call fails with 404 (as it should) | 19:36 |
| dansmith | anyway, I wasn't proposing a change to the test anyway, I was just more wondering if it was perhaps misplaced | 19:37 |
| gmann | i see, maybe delete image should stop that process too, notify nova | 19:37 |
| dansmith | but thinking about it from nova's perspective, it's a positive test | 19:37 |
| gmann | yeah | 19:38 |
| dansmith | well, it can't really because depending on ceph or non-ceph, nova might be doing the long running thing instead of glance | 19:38 |
| dansmith | anyway, I'm good, sorry for the noise | 19:38 |
| gmann | k | 19:40 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!