Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CI Unit tests #163

Merged
merged 20 commits into from
Jan 14, 2025
Merged

CI Unit tests #163

merged 20 commits into from
Jan 14, 2025

Conversation

pjrobertson
Copy link
Collaborator

This is a WIP branch, pushing so I can test whether the workflows run successfully or not.

Also includes the following:

@pjrobertson
Setup a basic framework for unit tests
30d423c 
Use 'python -m unittest' from the project root to run
@pjrobertson
Update pipfile for:
33e967c 
- pyopenssl==24.2.1
- youtube-dlp==2024.09.27
- numpy==2.1.3

Fixes building/local installs. Also fixes #155
@pjrobertson
Add Github CI test workflow
1bd0170
@pjrobertson
Merge branch 'main' into feat/unittest
ab9335b
@pjrobertson
Switch to pytest and use vcr for request storing
9dc4eb3
@pjrobertson
Merge branch 'main' into feat/unittest
e2bc84c
@pjrobertson
switch to pytest and pytest-recording
63973e2
@pjrobertson
Separate CI for download tests and core tests
2353f9d
@pjrobertson
Add unit tests for html_formatter, csv_db
bbef80d
@pjrobertson
Merge branch 'main' into feat/unittest
57eacdc
@pjrobertson
Flag tombstone tweets for twitter_syndication method
528b78d
@pjrobertson
Run expensive download tests once per week, on a month at 2:35pm
9b596e5 
(time is offset from the hour to alleviate high load on Github
@pjrobertson
Add unit test badges to readme
52f0649
@pjrobertson
Prettify pytest output
b9aea99
@pjrobertson
Merge branch 'main' into feat/unittest
bdfedfc
@pjrobertson
Add dev dependencies to poetry
ab2eb3c
@pjrobertson pjrobertson force-pushed the feat/unittest branch 4 times, most recently from b1463a8 to 398e8a4 Compare January 14, 2025 09:22
@pjrobertson
Switch to ubuntu-22.04 for CI tests
8f17a23 
An issue with oscrypto means it currently does not work on 24.04. Ref: wbond/oscrypto#78 (comment)
@pjrobertson pjrobertson changed the title [WIP] CI Unit tests CI Unit tests Jan 14, 2025
@pjrobertson
Copy link
Collaborator Author

pjrobertson commented Jan 14, 2025 •
edited
Loading

This PR should be good to go. Note:

✅ Core tests (not download) are passing - they run on all compatible python versions (3.10-3.12)
❌ Download tests are failing (because sensitive content tests are failing - they require logging in to see) - they run only on lowest supported python version 3.10
✅ Switched to pytest, but not using VCR for request recording (it doesn't work well with recording youtube-dlp requests, although I did make a PR on vcrpy about it
❌ I have not yet refactored the unit tests to tidy up URLs etc. I think that will be another PR, I am still trying to figure out the best structure for unit tests
✅ Testing virtualenvs are cached for quicker loading using the setup-python step with cache

Notes:

  • Core tests are run on every PR and push to main
  • Download tests are run once per week, on Monday at 2:35pm UTC (set to Monday to pick up any issues from the previous week)

erinhmclark
erinhmclark previously approved these changes Jan 14, 2025
View reviewed changes
Copy link
Collaborator

@erinhmclark erinhmclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! There are a few things in the tests which could be refactored since moving to using PyTest, but we do that in a future PR and get the CI up and running first.

tests/archivers/test_twitter_archiver.py Outdated Show resolved Hide resolved
tests/databases/test_csv_db.py Outdated Show resolved Hide resolved
.github/workflows/tests-download.yaml Show resolved Hide resolved
erinhmclark
erinhmclark approved these changes Jan 14, 2025
View reviewed changes
Copy link
Collaborator

@erinhmclark erinhmclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

tests/archivers/test_twitter_archiver.py

archiver_class = TwitterArchiver
config = {}
@pytest.mark.parametrize("url, expected", [
("https://t.co/yl3oOJatFp", "https://www.bellingcat.com/category/resources/"), # t.co URL
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could even add the comments into the parameterisation for speedier debugging:

    @pytest.mark.parametrize(
        "input_url, expected_url, description",
        [
            (
                "https://t.co/yl3oOJatFp",
                "https://www.bellingcat.com/category/resources/",
                "Should expand t.co URLs",
            ),
            
            ...
            
            assert archiver.sanitize_url(input_url) == expected_url, description

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Nice one. Missed this before I merged, I'll save it for later!

tests/archivers/test_twitter_archiver.py
self.assertValidResponseMetadata(
post,
"This month's Bellingchat Premium is with @KolinaKoltai. She reveals how she investigated a platform allowing users to create AI-generated child sexual abuse material and explains why it's crucial to investigate the people behind these services https://t.co/SfBUq0hSD0 https://t.co/rIHx0WlKp8",
datetime.datetime(2024, 12, 24, 13, 44, 46, tzinfo=datetime.timezone.utc)
)

@pytest.mark.xfail(reason="Currently failing, sensitive content requires logged in users/cookies - not yet implemented")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@pjrobertson pjrobertson merged commit eebd040 into main Jan 14, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erinhmclark erinhmclark erinhmclark approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pjrobertson @erinhmclark