Merge ~bryce/git-ubuntu:fix_derive_codename_from_series into git-ubuntu:master

Proposed by Bryce Harrington
Status: Superseded
Proposed branch: ~bryce/git-ubuntu:fix_derive_codename_from_series
Merge into: git-ubuntu:master
Diff against target: 195 lines (+150/-3)
2 files modified
gitubuntu/source_information.py (+21/-3)
gitubuntu/source_information_test.py (+129/-0)
Reviewer Review Type Date Requested Status
Robie Basak Needs Fixing
Server Team CI bot continuous-integration Approve
Review via email: mp+370035@code.staging.launchpad.net

Commit message

source_information: Fix ubuntu series lookup

Corrects an issue found by coverity:

  usd-importer/gitubuntu/source_information.py:107:
    copy_paste_error: "u_ddi" in "_ddi.codename" looks like a copy-paste error.
  usd-importer/gitubuntu/source_information.py:107:
    remediation: Should it say "u_udi" instead?

The code is erroneously using the Debian implementation of codename(),
which translates aliases ('unstable', 'testing', etc.) to the release
codenames ('sid', 'buster', etc.) Fortunately, if it doesn't match
anything it just returns the release name passed to it, which is exactly
what the Ubuntu implementation of codename() does.

Thus, even though the code is incorrect, it's behavior will always be
correct in practice. Thus this fix corrects a purely theoretical
problem, not one that would produce invalid behavior.

Tests and code documentation for derive_codename_from_series() are included.

Description of the change

These four patches should probably be squashed to land, although you may want to land the syntax error fix separately; I couldn't get pytest to function without it but it's an independent issue. I've left the patches split out for review convenience, but if they're landed this way patch #1 will introduce a test failure, that patch #3 fixes.

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:147037f0801dea8a85418407280768076045f3d8
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/102/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/102/rebuild

review: Approve (continuous-integration)
Revision history for this message
Robie Basak (racb) wrote :

...if they're landed this way patch #1 will introduce a test failure, that patch #3 fixes.

FYI, the way that we've been dealing with those is to use @pytest.mark.xfail when introducing a test known to be failing from that commit, and then to drop the decorator in the same commit as the fix.

I can arrange that when landing this.

Revision history for this message
Robie Basak (racb) wrote :

We reviewed this offline. Summary: looks great, some style suggestions. Bryce will fix up.

review: Needs Fixing
Revision history for this message
Bryce Harrington (bryce) wrote :

Review changes are squashed in and force pushed for re-review.

Revision history for this message
Robie Basak (racb) wrote :

I don't see an updated branch. The latest seems to be 147037f dated 2019-07-11. Please could you check?

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches