The exec migration test isn't run a whole test scenario.
This patch fixes it
Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
---
tests/acceptance/migration.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index a8367ca023..0365289cda 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -70,8 +70,8 @@ class Migration(Test):
@skipUnless(find_command('nc', default=False), "'nc' command not found")
def test_migration_with_exec(self):
- """
- The test works for both netcat-traditional and netcat-openbsd packages
- """
+ """The test works for both netcat-traditional and netcat-openbsd packages."""
free_port = self._get_free_port()
dest_uri = 'exec:nc -l localhost %u' % free_port
+ src_uri = 'exec:nc localhost %u' % free_port
+ self.do_migrate(dest_uri, src_uri)
--
2.21.1
Hi Oksana, v2 was https://www.mail-archive.com/qemu-devel@nongnu.org/msg682899.html, so this is v3. Please increment the version in the patch subject. You could also send a simple "ping" to the specific patch, instead of resending it. On 3/25/20 12:31 PM, Oksana Vohchana wrote: > The exec migration test isn't run a whole test scenario. > This patch fixes it > > Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com> v1 of this patch has already received reviewers tags (https://www.mail-archive.com/qemu-devel@nongnu.org/msg679629.html), please collect them and keep them when you resend the same patch: Fixes: 2e768cb682bf Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > --- > tests/acceptance/migration.py | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py > index a8367ca023..0365289cda 100644 > --- a/tests/acceptance/migration.py > +++ b/tests/acceptance/migration.py > @@ -70,8 +70,8 @@ class Migration(Test): > > @skipUnless(find_command('nc', default=False), "'nc' command not found") > def test_migration_with_exec(self): > - """ > - The test works for both netcat-traditional and netcat-openbsd packages > - """ > + """The test works for both netcat-traditional and netcat-openbsd packages.""" Btw why are you changing the comment style? > free_port = self._get_free_port() > dest_uri = 'exec:nc -l localhost %u' % free_port > + src_uri = 'exec:nc localhost %u' % free_port > + self.do_migrate(dest_uri, src_uri) > Alex, if there is no Python testing pullreq, can you take this patch via your testing tree? Thanks, Phil.
Hi Philippe Thanks for the review I have some comments On Wed, Mar 25, 2020 at 2:30 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > Hi Oksana, > > v2 was > https://www.mail-archive.com/qemu-devel@nongnu.org/msg682899.html, so > this is v3. Please increment the version in the patch subject. > > You could also send a simple "ping" to the specific patch, instead of > resending it. > > On 3/25/20 12:31 PM, Oksana Vohchana wrote: > > The exec migration test isn't run a whole test scenario. > > This patch fixes it > > > > Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com> > > v1 of this patch has already received reviewers tags > (https://www.mail-archive.com/qemu-devel@nongnu.org/msg679629.html), > please collect them and keep them when you resend the same patch: > I have reposted patch without this fix because this change isn't related to the series: https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06919.html Is it make sense to keep this fix as a separate patch? > Fixes: 2e768cb682bf > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > > > --- > > tests/acceptance/migration.py | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/tests/acceptance/migration.py > b/tests/acceptance/migration.py > > index a8367ca023..0365289cda 100644 > > --- a/tests/acceptance/migration.py > > +++ b/tests/acceptance/migration.py > > @@ -70,8 +70,8 @@ class Migration(Test): > > > > @skipUnless(find_command('nc', default=False), "'nc' command not > found") > > def test_migration_with_exec(self): > > - """ > > - The test works for both netcat-traditional and netcat-openbsd > packages > > - """ > > + """The test works for both netcat-traditional and > netcat-openbsd packages.""" > > Btw why are you changing the comment style? > I got failure in PEP257 > > free_port = self._get_free_port() > > dest_uri = 'exec:nc -l localhost %u' % free_port > > + src_uri = 'exec:nc localhost %u' % free_port > > + self.do_migrate(dest_uri, src_uri) > > > > Alex, if there is no Python testing pullreq, can you take this patch via > your testing tree? > > Thanks, > > Phil. > > Thanks
On 3/25/20 3:10 PM, Oksana Voshchana wrote: > Hi Philippe > Thanks for the review > I have some comments > > On Wed, Mar 25, 2020 at 2:30 PM Philippe Mathieu-Daudé > <philmd@redhat.com <mailto:philmd@redhat.com>> wrote: > > Hi Oksana, > > v2 was > https://www.mail-archive.com/qemu-devel@nongnu.org/msg682899.html, so > this is v3. Please increment the version in the patch subject. > > You could also send a simple "ping" to the specific patch, instead of > resending it. > > On 3/25/20 12:31 PM, Oksana Vohchana wrote: > > The exec migration test isn't run a whole test scenario. > > This patch fixes it > > > > Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com > <mailto:ovoshcha@redhat.com>> > > v1 of this patch has already received reviewers tags > (https://www.mail-archive.com/qemu-devel@nongnu.org/msg679629.html), > please collect them and keep them when you resend the same patch: > > I have reposted patch without this fix because this change isn't related > to the series: > https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06919.html > Is it make sense to keep this fix as a separate patch? > > Fixes: 2e768cb682bf > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com > <mailto:philmd@redhat.com>> > Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com > <mailto:wainersm@redhat.com>> > > > --- > > tests/acceptance/migration.py | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/tests/acceptance/migration.py > b/tests/acceptance/migration.py > > index a8367ca023..0365289cda 100644 > > --- a/tests/acceptance/migration.py > > +++ b/tests/acceptance/migration.py > > @@ -70,8 +70,8 @@ class Migration(Test): > > > > @skipUnless(find_command('nc', default=False), "'nc' > command not found") > > def test_migration_with_exec(self): > > - """ > > - The test works for both netcat-traditional and > netcat-openbsd packages > > - """ > > + """The test works for both netcat-traditional and > netcat-openbsd packages.""" > > Btw why are you changing the comment style? > > I got failure in PEP257 OK, next time please add comment in the patch description too. > > > > free_port = self._get_free_port() > > dest_uri = 'exec:nc -l localhost %u' % free_port > > + src_uri = 'exec:nc localhost %u' % free_port > > + self.do_migrate(dest_uri, src_uri) > > > > Alex, if there is no Python testing pullreq, can you take this patch > via > your testing tree? Cc'ing David since it is migration related. > > Thanks, > > Phil. > > Thanks
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote: > On 3/25/20 3:10 PM, Oksana Voshchana wrote: > > Hi Philippe > > Thanks for the review > > I have some comments > > > > On Wed, Mar 25, 2020 at 2:30 PM Philippe Mathieu-Daudé > > <philmd@redhat.com <mailto:philmd@redhat.com>> wrote: > > > > Hi Oksana, > > > > v2 was > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg682899.html, so > > this is v3. Please increment the version in the patch subject. > > > > You could also send a simple "ping" to the specific patch, instead of > > resending it. > > > > On 3/25/20 12:31 PM, Oksana Vohchana wrote: > > > The exec migration test isn't run a whole test scenario. > > > This patch fixes it > > > > > > Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com > > <mailto:ovoshcha@redhat.com>> > > > > v1 of this patch has already received reviewers tags > > (https://www.mail-archive.com/qemu-devel@nongnu.org/msg679629.html), > > please collect them and keep them when you resend the same patch: > > > > I have reposted patch without this fix because this change isn't related > > to the series: > > https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06919.html > > Is it make sense to keep this fix as a separate patch? > > > > Fixes: 2e768cb682bf > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com > > <mailto:philmd@redhat.com>> > > Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com > > <mailto:wainersm@redhat.com>> > > > > > --- > > > tests/acceptance/migration.py | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/tests/acceptance/migration.py > > b/tests/acceptance/migration.py > > > index a8367ca023..0365289cda 100644 > > > --- a/tests/acceptance/migration.py > > > +++ b/tests/acceptance/migration.py > > > @@ -70,8 +70,8 @@ class Migration(Test): > > > > > > @skipUnless(find_command('nc', default=False), "'nc' > > command not found") > > > def test_migration_with_exec(self): > > > - """ > > > - The test works for both netcat-traditional and > > netcat-openbsd packages > > > - """ > > > + """The test works for both netcat-traditional and > > netcat-openbsd packages.""" > > > > Btw why are you changing the comment style? > > > > I got failure in PEP257 > > OK, next time please add comment in the patch description too. > > > > > > > > free_port = self._get_free_port() > > > dest_uri = 'exec:nc -l localhost %u' % free_port > > > + src_uri = 'exec:nc localhost %u' % free_port > > > + self.do_migrate(dest_uri, src_uri) > > > > > > > Alex, if there is no Python testing pullreq, can you take this patch > > via > > your testing tree? > > Cc'ing David since it is migration related. I tend to leave the tests/acceptance to someone other than the migration tree; so feel free to take them via testing or trivial given the size. Dave > > > > Thanks, > > > > Phil. > > > > Thanks > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Hi David, On 3/25/20 4:15 PM, Dr. David Alan Gilbert wrote: > * Philippe Mathieu-Daudé (philmd@redhat.com) wrote: >> On 3/25/20 3:10 PM, Oksana Voshchana wrote: >>> Hi Philippe >>> Thanks for the review >>> I have some comments >>> >>> On Wed, Mar 25, 2020 at 2:30 PM Philippe Mathieu-Daudé >>> <philmd@redhat.com <mailto:philmd@redhat.com>> wrote: >>> >>> Hi Oksana, >>> >>> v2 was >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg682899.html, so >>> this is v3. Please increment the version in the patch subject. >>> >>> You could also send a simple "ping" to the specific patch, instead of >>> resending it. >>> >>> On 3/25/20 12:31 PM, Oksana Vohchana wrote: >>> > The exec migration test isn't run a whole test scenario. >>> > This patch fixes it >>> > >>> > Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com >>> <mailto:ovoshcha@redhat.com>> >>> >>> v1 of this patch has already received reviewers tags >>> (https://www.mail-archive.com/qemu-devel@nongnu.org/msg679629.html), >>> please collect them and keep them when you resend the same patch: >>> >>> I have reposted patch without this fix because this change isn't related >>> to the series: >>> https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06919.html >>> Is it make sense to keep this fix as a separate patch? >>> >>> Fixes: 2e768cb682bf >>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com >>> <mailto:philmd@redhat.com>> >>> Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com >>> <mailto:wainersm@redhat.com>> >>> >>> > --- >>> > tests/acceptance/migration.py | 6 +++--- >>> > 1 file changed, 3 insertions(+), 3 deletions(-) >>> > >>> > diff --git a/tests/acceptance/migration.py >>> b/tests/acceptance/migration.py >>> > index a8367ca023..0365289cda 100644 >>> > --- a/tests/acceptance/migration.py >>> > +++ b/tests/acceptance/migration.py >>> > @@ -70,8 +70,8 @@ class Migration(Test): >>> > >>> > @skipUnless(find_command('nc', default=False), "'nc' >>> command not found") >>> > def test_migration_with_exec(self): >>> > - """ >>> > - The test works for both netcat-traditional and >>> netcat-openbsd packages >>> > - """ >>> > + """The test works for both netcat-traditional and >>> netcat-openbsd packages.""" >>> >>> Btw why are you changing the comment style? >>> >>> I got failure in PEP257 >> >> OK, next time please add comment in the patch description too. >> >>> >>> >>> > free_port = self._get_free_port() >>> > dest_uri = 'exec:nc -l localhost %u' % free_port >>> > + src_uri = 'exec:nc localhost %u' % free_port >>> > + self.do_migrate(dest_uri, src_uri) >>> > >>> >>> Alex, if there is no Python testing pullreq, can you take this patch >>> via >>> your testing tree? >> >> Cc'ing David since it is migration related. > > I tend to leave the tests/acceptance to someone other than the migration > tree; so feel free to take them via testing or trivial given the size. Acceptance tests are in the same tests/acceptance directory for no particular reason. We thought they would share some common code but this code is in the python/qemu/ directory. Similarly to C Qtests stored in the tests/qtest, acceptance are in tests/acceptance. tests/qtest have various maintainers, when a maintainer has interest in a qtest (subsystem covered by the test), the test is added to the subsystem maintained section. I'd rather see the same with acceptance tests. For example with Oksana's test, I can review that it correctly uses the acceptance testing framework, and it doesn't break the rest of the tests, but I have no idea if it properly tests the migration features it aims to. Back to this particular patch, if Eduardo/Cleber ack I can send a pullreq for rc2. > > Dave > >>> >>> Thanks, >>> >>> Phil. >>> >>> Thanks >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
On 3/25/20 3:10 PM, Oksana Voshchana wrote: > Hi Philippe > Thanks for the review > I have some comments > > On Wed, Mar 25, 2020 at 2:30 PM Philippe Mathieu-Daudé > <philmd@redhat.com <mailto:philmd@redhat.com>> wrote: > > Hi Oksana, > > v2 was > https://www.mail-archive.com/qemu-devel@nongnu.org/msg682899.html, so > this is v3. Please increment the version in the patch subject. > > You could also send a simple "ping" to the specific patch, instead of > resending it. > > On 3/25/20 12:31 PM, Oksana Vohchana wrote: > > The exec migration test isn't run a whole test scenario. > > This patch fixes it > > > > Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com > <mailto:ovoshcha@redhat.com>> > > v1 of this patch has already received reviewers tags > (https://www.mail-archive.com/qemu-devel@nongnu.org/msg679629.html), > please collect them and keep them when you resend the same patch: > > I have reposted patch without this fix because this change isn't related > to the series: > https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06919.html > Is it make sense to keep this fix as a separate patch? As we are in freeze and this is a fix, it is fine to reply to your own patch with "Ping? As this is a fix, can we get this single patch merged for 5.0 please? Thanks!" You are responsible of tracking your own patches and ping them (every week) if they are ignored. > > Fixes: 2e768cb682bf > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com > <mailto:philmd@redhat.com>> > Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com > <mailto:wainersm@redhat.com>> > > > --- > > tests/acceptance/migration.py | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/tests/acceptance/migration.py > b/tests/acceptance/migration.py > > index a8367ca023..0365289cda 100644 > > --- a/tests/acceptance/migration.py > > +++ b/tests/acceptance/migration.py > > @@ -70,8 +70,8 @@ class Migration(Test): > > > > @skipUnless(find_command('nc', default=False), "'nc' > command not found") > > def test_migration_with_exec(self): > > - """ > > - The test works for both netcat-traditional and > netcat-openbsd packages > > - """ > > + """The test works for both netcat-traditional and > netcat-openbsd packages.""" > > Btw why are you changing the comment style? > > I got failure in PEP257 > > > > free_port = self._get_free_port() > > dest_uri = 'exec:nc -l localhost %u' % free_port > > + src_uri = 'exec:nc localhost %u' % free_port > > + self.do_migrate(dest_uri, src_uri) > > > > Alex, if there is no Python testing pullreq, can you take this patch > via > your testing tree? > > Thanks, > > Phil. > > Thanks
© 2016 - 2024 Red Hat, Inc.