There are two imports that need to be modified when running the iotests
under Python 3: One is StringIO, which no longer exists; instead, the
StringIO class comes from the io module, so import it from there. The
other is the ConfigParser, which has just been renamed to configparser.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/iotests.py | 8 ++++++--
tests/qemu-iotests/nbd-fault-injector.py | 7 +++++--
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 7ca94e9278..a64ea90fb4 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -683,13 +683,17 @@ def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[],
# We need to filter out the time taken from the output so that qemu-iotest
# can reliably diff the results against master output.
- import StringIO
+ if sys.version_info.major >= 3:
+ from io import StringIO
+ else:
+ from StringIO import StringIO
+
if debug:
output = sys.stdout
verbosity = 2
sys.argv.remove('-d')
else:
- output = StringIO.StringIO()
+ output = StringIO()
logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
index d45e2e0a6a..6b2d659dee 100755
--- a/tests/qemu-iotests/nbd-fault-injector.py
+++ b/tests/qemu-iotests/nbd-fault-injector.py
@@ -48,7 +48,10 @@ import sys
import socket
import struct
import collections
-import ConfigParser
+if sys.version_info.major >= 3:
+ import configparser
+else:
+ import ConfigParser as configparser
FAKE_DISK_SIZE = 8 * 1024 * 1024 * 1024 # 8 GB
@@ -225,7 +228,7 @@ def parse_config(config):
return rules
def load_rules(filename):
- config = ConfigParser.RawConfigParser()
+ config = configparser.RawConfigParser()
with open(filename, 'rt') as f:
config.readfp(f, filename)
return parse_config(config)
--
2.17.1
On 10/15/18 10:14 AM, Max Reitz wrote:
> There are two imports that need to be modified when running the iotests
> under Python 3: One is StringIO, which no longer exists; instead, the
> StringIO class comes from the io module, so import it from there. The
> other is the ConfigParser, which has just been renamed to configparser.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/iotests.py | 8 ++++++--
> tests/qemu-iotests/nbd-fault-injector.py | 7 +++++--
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 7ca94e9278..a64ea90fb4 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -683,13 +683,17 @@ def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[],
>
> # We need to filter out the time taken from the output so that qemu-iotest
> # can reliably diff the results against master output.
> - import StringIO
> + if sys.version_info.major >= 3:
> + from io import StringIO
> + else:
> + from StringIO import StringIO
> +
> if debug:
> output = sys.stdout
> verbosity = 2
> sys.argv.remove('-d')
> else:
> - output = StringIO.StringIO()
> + output = StringIO()
>
> logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
>
> diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
> index d45e2e0a6a..6b2d659dee 100755
> --- a/tests/qemu-iotests/nbd-fault-injector.py
> +++ b/tests/qemu-iotests/nbd-fault-injector.py
> @@ -48,7 +48,10 @@ import sys
> import socket
> import struct
> import collections
> -import ConfigParser
> +if sys.version_info.major >= 3:
> + import configparser
> +else:
> + import ConfigParser as configparser
>
> FAKE_DISK_SIZE = 8 * 1024 * 1024 * 1024 # 8 GB
>
> @@ -225,7 +228,7 @@ def parse_config(config):
> return rules
>
> def load_rules(filename):
> - config = ConfigParser.RawConfigParser()
> + config = configparser.RawConfigParser()
> with open(filename, 'rt') as f:
> config.readfp(f, filename)
> return parse_config(config)
>
This may be a type of culture clash (on my side, due to not enough QEMU
culture), but shouldn't this be applied before anything else on this series?
I mean, PATCH 1/9 is supposed to fix the reliability aspects of
nbd-fault-injector under Python 3, but without this patch, it won't
actually run on Python 3.
Regards,
- Cleber.
On Mon, Oct 15, 2018 at 02:59:28PM -0400, Cleber Rosa wrote:
>
>
> On 10/15/18 10:14 AM, Max Reitz wrote:
> > There are two imports that need to be modified when running the iotests
> > under Python 3: One is StringIO, which no longer exists; instead, the
> > StringIO class comes from the io module, so import it from there. The
> > other is the ConfigParser, which has just been renamed to configparser.
> >
> > Signed-off-by: Max Reitz <mreitz@redhat.com>
> > ---
> > tests/qemu-iotests/iotests.py | 8 ++++++--
> > tests/qemu-iotests/nbd-fault-injector.py | 7 +++++--
> > 2 files changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> > index 7ca94e9278..a64ea90fb4 100644
> > --- a/tests/qemu-iotests/iotests.py
> > +++ b/tests/qemu-iotests/iotests.py
> > @@ -683,13 +683,17 @@ def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[],
> >
> > # We need to filter out the time taken from the output so that qemu-iotest
> > # can reliably diff the results against master output.
> > - import StringIO
> > + if sys.version_info.major >= 3:
> > + from io import StringIO
> > + else:
> > + from StringIO import StringIO
> > +
> > if debug:
> > output = sys.stdout
> > verbosity = 2
> > sys.argv.remove('-d')
> > else:
> > - output = StringIO.StringIO()
> > + output = StringIO()
> >
> > logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
> >
> > diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
> > index d45e2e0a6a..6b2d659dee 100755
> > --- a/tests/qemu-iotests/nbd-fault-injector.py
> > +++ b/tests/qemu-iotests/nbd-fault-injector.py
> > @@ -48,7 +48,10 @@ import sys
> > import socket
> > import struct
> > import collections
> > -import ConfigParser
> > +if sys.version_info.major >= 3:
> > + import configparser
> > +else:
> > + import ConfigParser as configparser
> >
> > FAKE_DISK_SIZE = 8 * 1024 * 1024 * 1024 # 8 GB
> >
> > @@ -225,7 +228,7 @@ def parse_config(config):
> > return rules
> >
> > def load_rules(filename):
> > - config = ConfigParser.RawConfigParser()
> > + config = configparser.RawConfigParser()
> > with open(filename, 'rt') as f:
> > config.readfp(f, filename)
> > return parse_config(config)
> >
>
> This may be a type of culture clash (on my side, due to not enough QEMU
> culture), but shouldn't this be applied before anything else on this series?
>
> I mean, PATCH 1/9 is supposed to fix the reliability aspects of
> nbd-fault-injector under Python 3, but without this patch, it won't
> actually run on Python 3.
Both patches are required to make the code run on Python 3, and
they don't depend on each other. So I think the order doesn't
matter.
But I think the order chosen by Max is slightly more intuitive:
having explicit mentions of Python 3 in the code would be
confusing if we didn't fix the compatibility issues on patches
1-7 first.
--
Eduardo
On 15.10.18 20:59, Cleber Rosa wrote:
>
>
> On 10/15/18 10:14 AM, Max Reitz wrote:
>> There are two imports that need to be modified when running the iotests
>> under Python 3: One is StringIO, which no longer exists; instead, the
>> StringIO class comes from the io module, so import it from there. The
>> other is the ConfigParser, which has just been renamed to configparser.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> tests/qemu-iotests/iotests.py | 8 ++++++--
>> tests/qemu-iotests/nbd-fault-injector.py | 7 +++++--
>> 2 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 7ca94e9278..a64ea90fb4 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -683,13 +683,17 @@ def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[],
>>
>> # We need to filter out the time taken from the output so that qemu-iotest
>> # can reliably diff the results against master output.
>> - import StringIO
>> + if sys.version_info.major >= 3:
>> + from io import StringIO
>> + else:
>> + from StringIO import StringIO
>> +
>> if debug:
>> output = sys.stdout
>> verbosity = 2
>> sys.argv.remove('-d')
>> else:
>> - output = StringIO.StringIO()
>> + output = StringIO()
>>
>> logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
>>
>> diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
>> index d45e2e0a6a..6b2d659dee 100755
>> --- a/tests/qemu-iotests/nbd-fault-injector.py
>> +++ b/tests/qemu-iotests/nbd-fault-injector.py
>> @@ -48,7 +48,10 @@ import sys
>> import socket
>> import struct
>> import collections
>> -import ConfigParser
>> +if sys.version_info.major >= 3:
>> + import configparser
>> +else:
>> + import ConfigParser as configparser
>>
>> FAKE_DISK_SIZE = 8 * 1024 * 1024 * 1024 # 8 GB
>>
>> @@ -225,7 +228,7 @@ def parse_config(config):
>> return rules
>>
>> def load_rules(filename):
>> - config = ConfigParser.RawConfigParser()
>> + config = configparser.RawConfigParser()
>> with open(filename, 'rt') as f:
>> config.readfp(f, filename)
>> return parse_config(config)
>>
>
> This may be a type of culture clash (on my side, due to not enough QEMU
> culture), but shouldn't this be applied before anything else on this series?
>
> I mean, PATCH 1/9 is supposed to fix the reliability aspects of
> nbd-fault-injector under Python 3, but without this patch, it won't
> actually run on Python 3.
I don't mind, I followed no specific order. Well, patch 9 is patch 9
because it is the largest one, so I didn't want to discourage people
before the end of the series. :-)
Max
On Mon, Oct 15, 2018 at 04:14:52PM +0200, Max Reitz wrote:
> There are two imports that need to be modified when running the iotests
> under Python 3: One is StringIO, which no longer exists; instead, the
> StringIO class comes from the io module, so import it from there. The
> other is the ConfigParser, which has just been renamed to configparser.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/iotests.py | 8 ++++++--
> tests/qemu-iotests/nbd-fault-injector.py | 7 +++++--
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 7ca94e9278..a64ea90fb4 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -683,13 +683,17 @@ def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[],
>
> # We need to filter out the time taken from the output so that qemu-iotest
> # can reliably diff the results against master output.
> - import StringIO
> + if sys.version_info.major >= 3:
> + from io import StringIO
> + else:
> + from StringIO import StringIO
Considering that io.StringIO exists on Python 2.7, a comment
explaining why exactly it doesn't work would be nice.
But this shouldn't block this workaround, so:
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> +
> if debug:
> output = sys.stdout
> verbosity = 2
> sys.argv.remove('-d')
> else:
> - output = StringIO.StringIO()
> + output = StringIO()
>
> logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
>
> diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
> index d45e2e0a6a..6b2d659dee 100755
> --- a/tests/qemu-iotests/nbd-fault-injector.py
> +++ b/tests/qemu-iotests/nbd-fault-injector.py
> @@ -48,7 +48,10 @@ import sys
> import socket
> import struct
> import collections
> -import ConfigParser
> +if sys.version_info.major >= 3:
> + import configparser
> +else:
> + import ConfigParser as configparser
>
> FAKE_DISK_SIZE = 8 * 1024 * 1024 * 1024 # 8 GB
>
> @@ -225,7 +228,7 @@ def parse_config(config):
> return rules
>
> def load_rules(filename):
> - config = ConfigParser.RawConfigParser()
> + config = configparser.RawConfigParser()
> with open(filename, 'rt') as f:
> config.readfp(f, filename)
> return parse_config(config)
> --
> 2.17.1
>
--
Eduardo
On 10/15/18 5:17 PM, Eduardo Habkost wrote:
> On Mon, Oct 15, 2018 at 04:14:52PM +0200, Max Reitz wrote:
>> There are two imports that need to be modified when running the iotests
>> under Python 3: One is StringIO, which no longer exists; instead, the
>> StringIO class comes from the io module, so import it from there. The
>> other is the ConfigParser, which has just been renamed to configparser.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> tests/qemu-iotests/iotests.py | 8 ++++++--
>> tests/qemu-iotests/nbd-fault-injector.py | 7 +++++--
>> 2 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 7ca94e9278..a64ea90fb4 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -683,13 +683,17 @@ def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[],
>>
>> # We need to filter out the time taken from the output so that qemu-iotest
>> # can reliably diff the results against master output.
>> - import StringIO
>> + if sys.version_info.major >= 3:
>> + from io import StringIO
>> + else:
>> + from StringIO import StringIO
>
> Considering that io.StringIO exists on Python 2.7, a comment
> explaining why exactly it doesn't work would be nice.
>
Another possibility, that I find self explanatory:
import io
if sys.version_info.major >= 3:
output = io.StringIO()
else:
output = io.BytesIO()
- Cleber.
> But this shouldn't block this workaround, so:
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>
>
>> +
>> if debug:
>> output = sys.stdout
>> verbosity = 2
>> sys.argv.remove('-d')
>> else:
>> - output = StringIO.StringIO()
>> + output = StringIO()
>>
>> logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
>>
>> diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
>> index d45e2e0a6a..6b2d659dee 100755
>> --- a/tests/qemu-iotests/nbd-fault-injector.py
>> +++ b/tests/qemu-iotests/nbd-fault-injector.py
>> @@ -48,7 +48,10 @@ import sys
>> import socket
>> import struct
>> import collections
>> -import ConfigParser
>> +if sys.version_info.major >= 3:
>> + import configparser
>> +else:
>> + import ConfigParser as configparser
>>
>> FAKE_DISK_SIZE = 8 * 1024 * 1024 * 1024 # 8 GB
>>
>> @@ -225,7 +228,7 @@ def parse_config(config):
>> return rules
>>
>> def load_rules(filename):
>> - config = ConfigParser.RawConfigParser()
>> + config = configparser.RawConfigParser()
>> with open(filename, 'rt') as f:
>> config.readfp(f, filename)
>> return parse_config(config)
>> --
>> 2.17.1
>>
>
On Mon, Oct 15, 2018 at 08:05:02PM -0400, Cleber Rosa wrote: > > > On 10/15/18 5:17 PM, Eduardo Habkost wrote: > > On Mon, Oct 15, 2018 at 04:14:52PM +0200, Max Reitz wrote: > >> There are two imports that need to be modified when running the iotests > >> under Python 3: One is StringIO, which no longer exists; instead, the > >> StringIO class comes from the io module, so import it from there. The > >> other is the ConfigParser, which has just been renamed to configparser. > >> > >> Signed-off-by: Max Reitz <mreitz@redhat.com> > >> --- > >> tests/qemu-iotests/iotests.py | 8 ++++++-- > >> tests/qemu-iotests/nbd-fault-injector.py | 7 +++++-- > >> 2 files changed, 11 insertions(+), 4 deletions(-) > >> > >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > >> index 7ca94e9278..a64ea90fb4 100644 > >> --- a/tests/qemu-iotests/iotests.py > >> +++ b/tests/qemu-iotests/iotests.py > >> @@ -683,13 +683,17 @@ def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[], > >> > >> # We need to filter out the time taken from the output so that qemu-iotest > >> # can reliably diff the results against master output. > >> - import StringIO > >> + if sys.version_info.major >= 3: > >> + from io import StringIO > >> + else: > >> + from StringIO import StringIO > > > > Considering that io.StringIO exists on Python 2.7, a comment > > explaining why exactly it doesn't work would be nice. > > > > Another possibility, that I find self explanatory: > > import io > > if sys.version_info.major >= 3: > output = io.StringIO() > else: > output = io.BytesIO() Looks nice and clean. But I'm not sure all output sent to `output` when running with Python 2 will be byte strings. What if `unittest.TextTestRunner` tries to write unicode strings to `output`? -- Eduardo
On 16.10.18 02:12, Eduardo Habkost wrote: > On Mon, Oct 15, 2018 at 08:05:02PM -0400, Cleber Rosa wrote: >> >> >> On 10/15/18 5:17 PM, Eduardo Habkost wrote: >>> On Mon, Oct 15, 2018 at 04:14:52PM +0200, Max Reitz wrote: >>>> There are two imports that need to be modified when running the iotests >>>> under Python 3: One is StringIO, which no longer exists; instead, the >>>> StringIO class comes from the io module, so import it from there. The >>>> other is the ConfigParser, which has just been renamed to configparser. >>>> >>>> Signed-off-by: Max Reitz <mreitz@redhat.com> >>>> --- >>>> tests/qemu-iotests/iotests.py | 8 ++++++-- >>>> tests/qemu-iotests/nbd-fault-injector.py | 7 +++++-- >>>> 2 files changed, 11 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py >>>> index 7ca94e9278..a64ea90fb4 100644 >>>> --- a/tests/qemu-iotests/iotests.py >>>> +++ b/tests/qemu-iotests/iotests.py >>>> @@ -683,13 +683,17 @@ def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[], >>>> >>>> # We need to filter out the time taken from the output so that qemu-iotest >>>> # can reliably diff the results against master output. >>>> - import StringIO >>>> + if sys.version_info.major >= 3: >>>> + from io import StringIO >>>> + else: >>>> + from StringIO import StringIO >>> >>> Considering that io.StringIO exists on Python 2.7, a comment >>> explaining why exactly it doesn't work would be nice. Oh, it does exist? I didn't know. O:-) So I suppose it's because the test runner emits just normal strings, which in 2.x are byte strings; but io's StringIO always expects Unicode strings. StringIO, OTOH, accepts both (and returns Unicode strings once you put a Unicode string into it). >> Another possibility, that I find self explanatory: >> >> import io >> >> if sys.version_info.major >= 3: >> output = io.StringIO() >> else: >> output = io.BytesIO() > > Looks nice and clean. > > But I'm not sure all output sent to `output` when running with > Python 2 will be byte strings. What if `unittest.TextTestRunner` > tries to write unicode strings to `output`? Hm. It doesn't look this way to me in practice. And considering that it only really prints a test summary, I don't think there are edge cases where it behaves differently. So I think using BytesIO() in 2.x is better. Max
© 2016 - 2025 Red Hat, Inc.