Actually store latest known version cursor #873
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Zlopez
previously approved these changes
Dec 20, 2019
Codecov Report
@@ Coverage Diff @@
## master #873 +/- ##
==========================================
+ Coverage 97.32% 97.32% +<.01%
==========================================
Files 61 61
Lines 3696 3697 +1
Branches 498 499 +1
==========================================
+ Hits 3597 3598 +1
Misses 58 58
Partials 41 41
Continue to review full report at Codecov.
|
|
Hang on, aside from the news file I need to figure out what the issue with coverage is here. |
nphilipp
force-pushed
the
master--github-cursors-electric-boogaloo
branch
2 times, most recently
from
c743991 to
cefecea
Compare
|
Could you squash the commits, when you think this is ready for merge? |
Signed-off-by: Nils Philippsen <[email protected]>
nphilipp
force-pushed
the
master--github-cursors-electric-boogaloo
branch
from
cefecea to
584e355
Compare
Of course. |
nphilipp
force-pushed
the
master--github-cursors-electric-boogaloo
branch
from
584e355 to
bdfec9f
Compare
|
And this is it with the commits squashed, except the one fixing an unrelated typo. |
nphilipp
force-pushed
the
master--github-cursors-electric-boogaloo
branch
from
bdfec9f to
9b612dc
Compare
Zlopez
previously approved these changes
Dec 20, 2019
|
This pull request introduces 1 alert when merging 9b612dc into 4d9036b - view on LGTM.com new alerts:
|
Unit tests only help if you test all involved units. ;) Additionally: - Rename Project.latest_known_cursor to latest_version_cursor. This makes it more obvious that the latest version and cursor belong together. - Improve GitHub JSON tests - split up test_parse_json() and reuse test project between them - add missing cursor info to response - use .assertRaises as a context manager - Fail if cursor is missing from GitHub API response. With the GraphQL API, responses are guaranteed to contain all the queried fields. Previously, we worked around it missing because we had old mock data from before it was introduced. Since we refreshed the mock data, we can be more strict about what to expect as responses. - Make dropping column work on SQLite with previous migration script. SQLite doesn't support dropping columns, so use Alembic's batch operation to recreate the table with the column dropped. See https://alembic.sqlalchemy.org/en/latest/batch.html for details. - Rename the news file to refer to the right PR Fixes: fedora-infra#589 Signed-off-by: Nils Philippsen <[email protected]>
nphilipp
force-pushed
the
master--github-cursors-electric-boogaloo
branch
from
9b612dc to
f772911
Compare
|
That's what I get for committing without running flake8 myself, sorry. 😉 |
|
Yeah, it's good to have it in tests |
Zlopez
approved these changes
Dec 20, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a little embarassing but #863 doesn't really store the latest known cursor because of a bug in
check_project_release(). Additionally to fixing this, improve the GitHub tests and migration script and be a little more strict about accepted API responses.