Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 889330 - Improve test coverage for news functionality
Summary: Improve test coverage for news functionality
Status: RESOLVED FIXED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Core (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords: InVCS, PullRequest
Depends on: 891001
Blocks: 920241
  Show dependency tree
 
Reported: 2023-01-02 05:34 UTC by Sam James
Modified: 2023-12-18 05:05 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sam James archtester Gentoo Infrastructure gentoo-dev Security 2023-01-02 05:34:48 UTC
We've had far too many issues with news items recently, so I think better coverage of the various conditions (Display-If-*) is in order.

commit 0e56f99b34939bf38dcfc0f9edf43a51f6ccf3fe
Author: Sam James <sam@gentoo.org>
Date:   Mon Jan 2 04:52:54 2023 +0000

    news: simplify isRelevant() check

    Hopefully a bit easier to follow now. I'm also not convinced
    it was right before, as the previous required every restriction.checkRestriction(...)
    to be true, while the original code only required *one* to be true per
    restriction.

    Thanks to kurly for noticing a recent news item wasn't showing up.

    Fixes: 9e24d0143450628f334cdb62e579efafd1bfd2ba
    Fixes: 1ffaa70544f34e93df24c0a175105a900bf272bf
    Signed-off-by: Sam James <sam@gentoo.org>

commit f1d98b6dc36ff2b47c36427c9938999320352eb4
Author: Sam James <sam@gentoo.org>
Date:   Mon Jan 2 04:47:11 2023 +0000

    news: fix value of profiles_base

    This fixes matching profile paths in Display-If-Profile in news items.

    bad: kwargs['profile']='var/db/repos/gentoo/profiles/default/linux/amd64/17.1/hardened', self.profile='default/linux/amd64/17.1/hardened'
    good: kwargs['profile']='default/linux/amd64/17.1/hardened', self.profile='default/linux/amd64/17.1/hardened'

    os.path.join() treats paths differently based on the components given:
    'os.path.join(portdir, "profiles", os.path.sep)' passes 3 different paths,
    whereas before 64d84ce2d9a333e83e2a5fba5e7ec95f936959e7, and now, we concat.
    profiles and os.path.sep first so that further splitting isn't carried out.

    Thanks to kurly for noticing a recent news item wasn't showing up.

    Fixes: 64d84ce2d9a333e83e2a5fba5e7ec95f936959e7
    Signed-off-by: Sam James <sam@gentoo.org>

commit 1ffaa70544f34e93df24c0a175105a900bf272bf
Author: Sam James <sam@gentoo.org>
Date:   Mon Jul 11 05:50:24 2022 +0100

    news: fix isRelevant check

    Manifested as all news items being shown, even if
    (very) irrelevant to the running system (e.g.
    different arch, packages not installed, ...).

    I think the distinction here is that with the previous state,
    we'd end up with _only_ Trues, or nothing (an element
    would be omitted), whereas this commit means we end
    up considering a possible mixed sequence.

    Closes: https://bugs.gentoo.org/857669
    Fixes: 9e24d0143450628f334cdb62e579efafd1bfd2ba
    Reported-by: kurly
    Signed-off-by: Sam James <sam@gentoo.org>
Comment 1 Larry the Git Cow gentoo-dev 2023-02-18 10:13:51 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/portage.git/commit/?id=f6f7f8138b09c079bff53ea0869c0d5af26caf4c

commit f6f7f8138b09c079bff53ea0869c0d5af26caf4c
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-02-18 09:37:12 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-02-18 10:13:46 +0000

    tests: news: reduce boilerplate further
    
    Bug: https://bugs.gentoo.org/889330
    Closes: https://github.com/gentoo/portage/pull/989
    Signed-off-by: Sam James <sam@gentoo.org>

 lib/portage/dbapi/virtual.py            |   2 +-
 lib/portage/tests/news/test_NewsItem.py | 173 ++++++++++----------------------
 2 files changed, 55 insertions(+), 120 deletions(-)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=5718444c3d8f21cce336c744c9afe3901c22c5d8

commit 5718444c3d8f21cce336c744c9afe3901c22c5d8
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-02-16 08:19:42 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-02-18 10:13:35 +0000

    tests: news: add test cases for previous news regressions
    
    These tests would've caught the 3 previous regressions we had in the news feature
    over the last year.
    
    Verified by reverting each of the relevant fix commits (see below) and confirming
    the relevant tests start to fail then pass again once the fix is cherry-picked in isolation.
    
    See: 0e56f99b34939bf38dcfc0f9edf43a51f6ccf3fe
    See: f1d98b6dc36ff2b47c36427c9938999320352eb4
    See: 1ffaa70544f34e93df24c0a175105a900bf272bf
    Bug: https://bugs.gentoo.org/857669
    Bug: https://bugs.gentoo.org/889330
    Signed-off-by: Sam James <sam@gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 329 +++++++++++++++++++++++++++++---
 1 file changed, 302 insertions(+), 27 deletions(-)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=1ca9ff926ae0cc7778041d21999c59340bd2d5cb

commit 1ca9ff926ae0cc7778041d21999c59340bd2d5cb
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-02-16 08:35:58 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-02-18 10:13:35 +0000

    tests: news: use mocked NewsItem
    
    This eliminates a lot of boilerplate from each test.
    
    Bug: https://bugs.gentoo.org/889330
    Signed-off-by: Sam James <sam@gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 124 ++++++++++++++------------------
 1 file changed, 54 insertions(+), 70 deletions(-)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=bed3311d84455ca49b45dc3146ecaf74d6ee8dc1

commit bed3311d84455ca49b45dc3146ecaf74d6ee8dc1
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-02-16 08:09:43 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-02-18 10:13:35 +0000

    tests: news: drop usage of incomplete testdbapi, port to fakedbapi
    
    It's been TODO for many years (see a4acda03bac43cc972dfaf9fda4d5210860d3d93)
    and it covered up a problem with Display-If-Installed's test not actually
    asserting if the package was installed or not.
    
    Now e.g. dbapi.match() gives a proper result.
    
    Bug: https://bugs.gentoo.org/889330
    Signed-off-by: Sam James <sam@gentoo.org>

 lib/portage/dbapi/virtual.py            | 19 -------------------
 lib/portage/tests/news/test_NewsItem.py | 10 ++++------
 2 files changed, 4 insertions(+), 25 deletions(-)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=b7bd20a83676fa29ac92a9790cee84c68148beb0

commit b7bd20a83676fa29ac92a9790cee84c68148beb0
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-02-16 08:02:14 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-02-18 10:13:35 +0000

    tests: news: add failing Display-If-Installed test
    
    This shows that our current Display-If-Installed test
    doesn't work properly, as it'll pass with any package
    value.
    
    Bug: https://bugs.gentoo.org/889330
    Signed-off-by: Sam James <sam@gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=ebf78db6d9bb72f5ca6538ad5cc94ec297f9aa2f

commit ebf78db6d9bb72f5ca6538ad5cc94ec297f9aa2f
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-02-16 07:42:31 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-02-18 10:13:35 +0000

    tests: news: add trivial no-filter test
    
    Add a basic test case for when no filter fields are used
    (no Display-If-*).
    
    Bug: https://bugs.gentoo.org/889330
    Signed-off-by: Sam James <sam@gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 7 +++++++
 1 file changed, 7 insertions(+)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=b369e8296fcc802178f5e178ccc0e43307c789bd

commit b369e8296fcc802178f5e178ccc0e43307c789bd
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-02-16 07:41:19 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-02-18 10:13:34 +0000

    tests: news: mention GLEP 42 explicitly
    
    Bug: https://bugs.gentoo.org/889330
    Signed-off-by: Sam James <sam@gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 3 +++
 1 file changed, 3 insertions(+)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=23b0bfae026a7bbb31e544eed9b8346ef85fc610

commit 23b0bfae026a7bbb31e544eed9b8346ef85fc610
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-02-16 07:35:05 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-02-18 10:13:34 +0000

    tests: news: always check for validity of generated news item
    
    Previously, we'd ignore invalid news items in these tests
    because they're intended to check for the respective
    'relevance' field. Fix that.
    
    Bug: https://bugs.gentoo.org/889330
    Signed-off-by: Sam James <sam@gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 3 +++
 1 file changed, 3 insertions(+)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=c074e090e057a6d4303b492493c8cd5e9a180107

commit c074e090e057a6d4303b492493c8cd5e9a180107
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-02-16 07:30:08 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-02-18 10:13:34 +0000

    tests: news: use templates for mock news items
    
    This is some prep work to modernise the tests a bit
    by using dataclasses and idiomatic Python string/template
    substitution.
    
    The key point here is the changes facilitate testing
    with various combinations of fields, including multiple
    e.g. Display-If-Installed, which we couldn't do cleanly
    before.
    
    Bug: https://bugs.gentoo.org/889330
    Signed-off-by: Sam James <sam@gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 149 ++++++++++++++++++++++++--------
 1 file changed, 111 insertions(+), 38 deletions(-)
Comment 2 Larry the Git Cow gentoo-dev 2023-02-18 10:15:34 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/portage.git/commit/?id=cac983e4bf4985aecb45a776e0a3795bb78d035b

commit cac983e4bf4985aecb45a776e0a3795bb78d035b
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-02-18 10:15:21 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-02-18 10:15:21 +0000

    NEWS: mention test changes for news
    
    Bug: https://bugs.gentoo.org/889330
    Signed-off-by: Sam James <sam@gentoo.org>

 NEWS | 2 ++
 1 file changed, 2 insertions(+)
Comment 3 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2023-02-18 10:29:56 UTC
One more thing I forgot to note: the old tests for Display-If-Keyword didn't work because they tried to substitute 'Display-If-Arch' (which doesn't exist) in the template. Fixed now too.
Comment 4 Larry the Git Cow gentoo-dev 2023-02-26 22:01:36 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=cd8ade10313d72cb0e3dd2229df02e0ea8681daa

commit cd8ade10313d72cb0e3dd2229df02e0ea8681daa
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-02-26 22:01:01 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-02-26 22:01:10 +0000

    sys-apps/portage: add 3.0.45
    
    Bug: https://bugs.gentoo.org/891001
    Bug: https://bugs.gentoo.org/889330
    Bug: https://bugs.gentoo.org/890777
    Bug: https://bugs.gentoo.org/891391
    Bug: https://bugs.gentoo.org/893638
    Bug: https://bugs.gentoo.org/795825
    Bug: https://bugs.gentoo.org/884869
    Bug: https://bugs.gentoo.org/888585
    Bug: https://bugs.gentoo.org/892651
    Bug: https://bugs.gentoo.org/895526
    Signed-off-by: Sam James <sam@gentoo.org>

 sys-apps/portage/Manifest              |   1 +
 sys-apps/portage/portage-3.0.45.ebuild | 288 +++++++++++++++++++++++++++++++++
 2 files changed, 289 insertions(+)