| Message ID | 20240408143935.23628-1-fnordahl@ubuntu.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] tests: Fix compatibility issue with Python 3.12 in vlog.at. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 4/8/24 16:39, Frode Nordahl wrote: > The vlog - Python3 test makes use of output from Python > Tracebacks in its assertion. > > In Python 3.12 a line with tophat (``^``) markers is added below > the assert line, which makes the test fail. Hmm. Are you sure it's 3.12? I believe I did run tests with 3.12 a few times at some point and didn't have this issue. Bets regards, Ilya Maximets. > > Strip lines starting whith whitespace and otherwise only > containing one or more occurence of the ``^`` character from the > output before performing the test assertions. > > Note that the extra pair of brackets (``[]``) is required to make > m4 pass the expected string (``[[:space:]]``) to sed. > > Reported-at: https://launchpad.net/bugs/2060434 > Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com> > --- > tests/vlog.at | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/vlog.at b/tests/vlog.at > index 785014956..9a0c7d787 100644 > --- a/tests/vlog.at > +++ b/tests/vlog.at > @@ -8,6 +8,7 @@ AT_CHECK([$PYTHON3 $srcdir/test-vlog.py --log-file log_file \ > > AT_CHECK([sed -e 's/.*-.*-.*T..:..:..Z |//' \ > -e 's/File ".*", line [[0-9]][[0-9]]*,/File <name>, line <number>,/' \ > +-e '/[[[:space:]]]\+^\+/d' \ > stderr_log], [0], [dnl > 0 | module_0 | EMER | emergency > 1 | module_0 | ERR | error
On Mon, Apr 8, 2024 at 4:44 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 4/8/24 16:39, Frode Nordahl wrote: > > The vlog - Python3 test makes use of output from Python > > Tracebacks in its assertion. > > > > In Python 3.12 a line with tophat (``^``) markers is added below > > the assert line, which makes the test fail. > > Hmm. Are you sure it's 3.12? > > I believe I did run tests with 3.12 a few times at some point > and didn't have this issue. I guess I should have spelled out the specific point release in use, we see it in Ubuntu with Python 3.12.2 [0]. 0: https://launchpad.net/ubuntu/+source/python3.12/3.12.2-5ubuntu3
On 4/8/24 16:48, Frode Nordahl wrote: > On Mon, Apr 8, 2024 at 4:44 PM Ilya Maximets <i.maximets@ovn.org> wrote: >> >> On 4/8/24 16:39, Frode Nordahl wrote: >>> The vlog - Python3 test makes use of output from Python >>> Tracebacks in its assertion. >>> >>> In Python 3.12 a line with tophat (``^``) markers is added below >>> the assert line, which makes the test fail. >> >> Hmm. Are you sure it's 3.12? >> >> I believe I did run tests with 3.12 a few times at some point >> and didn't have this issue. > > I guess I should have spelled out the specific point release in use, > we see it in Ubuntu with Python 3.12.2 [0]. > > 0: https://launchpad.net/ubuntu/+source/python3.12/3.12.2-5ubuntu3 > Even 3.12.2 doesn't seem to explain the situation. 3.12.2 in Fedora 41 doesn't have this issue. Moreover, these is something fishy here. On Fedora I don't have any '^' markers in the output, but on Ubuntu I see them. AFAICT, recent changes in 3.12 branch should have changed the underlining from a single '^' to multiple. But not from having none at all to multiple. Also, the same should be true for python starting from 3.10: https://github.com/python/cpython/issues/116563 That's strange. To be clear, I'm not opposed to a change, I'm just a little puzzled on what is going on here, as it doesn't seem to be related much to the python version. Best regards, Ilya Maximets.
On Mon, Apr 8, 2024 at 9:14 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 4/8/24 16:48, Frode Nordahl wrote: > > On Mon, Apr 8, 2024 at 4:44 PM Ilya Maximets <i.maximets@ovn.org> wrote: > >> > >> On 4/8/24 16:39, Frode Nordahl wrote: > >>> The vlog - Python3 test makes use of output from Python > >>> Tracebacks in its assertion. > >>> > >>> In Python 3.12 a line with tophat (``^``) markers is added below > >>> the assert line, which makes the test fail. > >> > >> Hmm. Are you sure it's 3.12? > >> > >> I believe I did run tests with 3.12 a few times at some point > >> and didn't have this issue. > > > > I guess I should have spelled out the specific point release in use, > > we see it in Ubuntu with Python 3.12.2 [0]. > > > > 0: https://launchpad.net/ubuntu/+source/python3.12/3.12.2-5ubuntu3 > > > > Even 3.12.2 doesn't seem to explain the situation. > > 3.12.2 in Fedora 41 doesn't have this issue. Moreover, these is something > fishy here. On Fedora I don't have any '^' markers in the output, but on > Ubuntu I see them. AFAICT, recent changes in 3.12 branch should have changed > the underlining from a single '^' to multiple. But not from having none at > all to multiple. Also, the same should be true for python starting from 3.10: > https://github.com/python/cpython/issues/116563 > That's strange. > > To be clear, I'm not opposed to a change, I'm just a little puzzled on what > is going on here, as it doesn't seem to be related much to the python version. Thanks for checking, and no issue at all, I was also a bit confused as it appeared to change between two package patch versions, and I landed on something Python 3.12.2 to be most likely. I'll do an extra round of due diligence so that we are sure we understand what's going on. -- Frode Nordahl > Best regards, Ilya Maximets.
On Mon, Apr 8, 2024 at 9:18 PM Frode Nordahl <fnordahl@ubuntu.com> wrote: > > On Mon, Apr 8, 2024 at 9:14 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > > > On 4/8/24 16:48, Frode Nordahl wrote: > > > On Mon, Apr 8, 2024 at 4:44 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > >> > > >> On 4/8/24 16:39, Frode Nordahl wrote: > > >>> The vlog - Python3 test makes use of output from Python > > >>> Tracebacks in its assertion. > > >>> > > >>> In Python 3.12 a line with tophat (``^``) markers is added below > > >>> the assert line, which makes the test fail. > > >> > > >> Hmm. Are you sure it's 3.12? > > >> > > >> I believe I did run tests with 3.12 a few times at some point > > >> and didn't have this issue. > > > > > > I guess I should have spelled out the specific point release in use, > > > we see it in Ubuntu with Python 3.12.2 [0]. > > > > > > 0: https://launchpad.net/ubuntu/+source/python3.12/3.12.2-5ubuntu3 > > > > > > > Even 3.12.2 doesn't seem to explain the situation. > > > > 3.12.2 in Fedora 41 doesn't have this issue. Moreover, these is something > > fishy here. On Fedora I don't have any '^' markers in the output, but on > > Ubuntu I see them. AFAICT, recent changes in 3.12 branch should have changed > > the underlining from a single '^' to multiple. But not from having none at > > all to multiple. Also, the same should be true for python starting from 3.10: > > https://github.com/python/cpython/issues/116563 > > That's strange. > > > > To be clear, I'm not opposed to a change, I'm just a little puzzled on what > > is going on here, as it doesn't seem to be related much to the python version. > > Thanks for checking, and no issue at all, I was also a bit confused as > it appeared to change between two package patch versions, and I landed > on something Python 3.12.2 to be most likely. > > I'll do an extra round of due diligence so that we are sure we > understand what's going on. I've tracked the change of behavior down to this upstream fix: https://github.com/python/cpython/issues/116034 Which is included in the Debian/Ubuntu packages. AFAICT from the discussion on that issue, they are essentially making Python 3.11 and 3.12 behave like Python 3.13. So, I guess for OVS, we could make the commit subject address Python 3.13 compatibility with a note about those backports for 3.12 and 3.11 in the message? WDYT? > -- > Frode Nordahl > > > Best regards, Ilya Maximets.
On 4/8/24 22:10, Frode Nordahl wrote: > On Mon, Apr 8, 2024 at 9:18 PM Frode Nordahl <fnordahl@ubuntu.com> wrote: >> >> On Mon, Apr 8, 2024 at 9:14 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>> >>> On 4/8/24 16:48, Frode Nordahl wrote: >>>> On Mon, Apr 8, 2024 at 4:44 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>>>> >>>>> On 4/8/24 16:39, Frode Nordahl wrote: >>>>>> The vlog - Python3 test makes use of output from Python >>>>>> Tracebacks in its assertion. >>>>>> >>>>>> In Python 3.12 a line with tophat (``^``) markers is added below >>>>>> the assert line, which makes the test fail. >>>>> >>>>> Hmm. Are you sure it's 3.12? >>>>> >>>>> I believe I did run tests with 3.12 a few times at some point >>>>> and didn't have this issue. >>>> >>>> I guess I should have spelled out the specific point release in use, >>>> we see it in Ubuntu with Python 3.12.2 [0]. >>>> >>>> 0: https://launchpad.net/ubuntu/+source/python3.12/3.12.2-5ubuntu3 >>>> >>> >>> Even 3.12.2 doesn't seem to explain the situation. >>> >>> 3.12.2 in Fedora 41 doesn't have this issue. Moreover, these is something >>> fishy here. On Fedora I don't have any '^' markers in the output, but on >>> Ubuntu I see them. AFAICT, recent changes in 3.12 branch should have changed >>> the underlining from a single '^' to multiple. But not from having none at >>> all to multiple. Also, the same should be true for python starting from 3.10: >>> https://github.com/python/cpython/issues/116563 >>> That's strange. >>> >>> To be clear, I'm not opposed to a change, I'm just a little puzzled on what >>> is going on here, as it doesn't seem to be related much to the python version. >> >> Thanks for checking, and no issue at all, I was also a bit confused as >> it appeared to change between two package patch versions, and I landed >> on something Python 3.12.2 to be most likely. >> >> I'll do an extra round of due diligence so that we are sure we >> understand what's going on. > > I've tracked the change of behavior down to this upstream fix: > https://github.com/python/cpython/issues/116034 > > Which is included in the Debian/Ubuntu packages. > > AFAICT from the discussion on that issue, they are essentially making > Python 3.11 and 3.12 behave like Python 3.13. > > So, I guess for OVS, we could make the commit subject address Python > 3.13 compatibility with a note about those backports for 3.12 and 3.11 > in the message? > > WDYT? Ack. Sounds good to me. Thanks for finding the root cause! For the patch itself, I'm not sure if '/[[[:space:]]]\+^\+/d' sufficiently protects us from false positives, i.e. if we're not matching on a line start ($), then there is probably not much sense in matching on spaces either. I'd suggest to add a '$' or just match on any line containing the '^'. We do not expect any lines like that in the test. Best regards, Ilya Maximets.
diff --git a/tests/vlog.at b/tests/vlog.at index 785014956..9a0c7d787 100644 --- a/tests/vlog.at +++ b/tests/vlog.at @@ -8,6 +8,7 @@ AT_CHECK([$PYTHON3 $srcdir/test-vlog.py --log-file log_file \ AT_CHECK([sed -e 's/.*-.*-.*T..:..:..Z |//' \ -e 's/File ".*", line [[0-9]][[0-9]]*,/File <name>, line <number>,/' \ +-e '/[[[:space:]]]\+^\+/d' \ stderr_log], [0], [dnl 0 | module_0 | EMER | emergency 1 | module_0 | ERR | error
The vlog - Python3 test makes use of output from Python Tracebacks in its assertion. In Python 3.12 a line with tophat (``^``) markers is added below the assert line, which makes the test fail. Strip lines starting whith whitespace and otherwise only containing one or more occurence of the ``^`` character from the output before performing the test assertions. Note that the extra pair of brackets (``[]``) is required to make m4 pass the expected string (``[[:space:]]``) to sed. Reported-at: https://launchpad.net/bugs/2060434 Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com> --- tests/vlog.at | 1 + 1 file changed, 1 insertion(+)