| Message ID | 20210329132632.68901-3-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests/297: Cover tests/ | expand |
On Mon, Mar 29, 2021 at 10:28 AM Max Reitz <mreitz@redhat.com> wrote: > > pylint complains that discards1_sha256 and all_discards_sha256 are first > set in non-__init__ methods. Let's make it happy. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 3 +++ > 1 file changed, 3 insertions(+) > Reviewed-by: Willian Rampazzo <willianr@redhat.com>
29.03.2021 16:26, Max Reitz wrote: > pylint complains that discards1_sha256 and all_discards_sha256 are first > set in non-__init__ methods. Let's make it happy. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test > index 584062b412..013e94fc39 100755 > --- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test > +++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test > @@ -76,6 +76,9 @@ def check_bitmaps(vm, count): > > > class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): > + discards1_sha256 = None > + all_discards_sha256 = None > + > def tearDown(self): > if debug: > self.vm_a_events += self.vm_a.get_qmp_events() > I'd prefer not making them class-variables. I think initializing them in setUp should work (as a lot of other variables are initialized in setUp() and pylint doesn't complain). And better thing is return it together with event_resume from start_postcopy(), as actually it's a kind of result of the function.
On 30.03.21 18:47, Vladimir Sementsov-Ogievskiy wrote: > 29.03.2021 16:26, Max Reitz wrote: >> pylint complains that discards1_sha256 and all_discards_sha256 are first >> set in non-__init__ methods. Let's make it happy. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test >> b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test >> index 584062b412..013e94fc39 100755 >> --- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test >> +++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test >> @@ -76,6 +76,9 @@ def check_bitmaps(vm, count): >> class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): >> + discards1_sha256 = None >> + all_discards_sha256 = None >> + >> def tearDown(self): >> if debug: >> self.vm_a_events += self.vm_a.get_qmp_events() >> > > I'd prefer not making them class-variables. I think initializing them in > setUp should work (as a lot of other variables are initialized in > setUp() and pylint doesn't complain). And better thing is return it > together with event_resume from start_postcopy(), as actually it's a > kind of result of the function. Oh, that sounds good. Is a list fine, i.e. return (event_resume, discards1_sha256, all_discards_sha256)? (We could also make it an object. I don’t know what Python prefers. :)) Max
30.03.2021 20:18, Max Reitz wrote: > On 30.03.21 18:47, Vladimir Sementsov-Ogievskiy wrote: >> 29.03.2021 16:26, Max Reitz wrote: >>> pylint complains that discards1_sha256 and all_discards_sha256 are first >>> set in non-__init__ methods. Let's make it happy. >>> >>> Signed-off-by: Max Reitz <mreitz@redhat.com> >>> --- >>> tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test >>> index 584062b412..013e94fc39 100755 >>> --- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test >>> +++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test >>> @@ -76,6 +76,9 @@ def check_bitmaps(vm, count): >>> class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): >>> + discards1_sha256 = None >>> + all_discards_sha256 = None >>> + >>> def tearDown(self): >>> if debug: >>> self.vm_a_events += self.vm_a.get_qmp_events() >>> >> >> I'd prefer not making them class-variables. I think initializing them in setUp should work (as a lot of other variables are initialized in setUp() and pylint doesn't complain). And better thing is return it together with event_resume from start_postcopy(), as actually it's a kind of result of the function. > > Oh, that sounds good. Is a list fine, i.e. return (event_resume, discards1_sha256, all_discards_sha256)? > I think list is fine for now. If in future we'll want more logic in this place, we'll refactor it to some object or something.
diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test index 584062b412..013e94fc39 100755 --- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test +++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test @@ -76,6 +76,9 @@ def check_bitmaps(vm, count): class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): + discards1_sha256 = None + all_discards_sha256 = None + def tearDown(self): if debug: self.vm_a_events += self.vm_a.get_qmp_events()
pylint complains that discards1_sha256 and all_discards_sha256 are first set in non-__init__ methods. Let's make it happy. Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 3 +++ 1 file changed, 3 insertions(+)