Skip to content

Add tests for Conferences script #404

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.

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

Merged
merged 12 commits into from
Sep 1, 2024

Conversation

dragid10
Copy link
Member

@dragid10 dragid10 commented Aug 30, 2024

Fixes #384

TLDR Summary:

This PR does 3 things

  1. Adds tests for the logic introduced in Ensure the conference URL always has a scheme #379
  2. Refactors the Conferences script into testable functions
  3. Adds wait time to the CI and tests so that playwright has time to get itself together 😄

What's new:

  • Added 3 unit tests in tests/test.py:
    • When the URL in the issue is valid (has a scheme)
    • When the URL in the issue is valid (but missing a scheme)
    • When there is no URL in the issue at all
  • Introduced startup wait time after serving the local site and before running tests: This was one I actually did hit a couple of times while testing locally. Playwright was very eager to just start testing things immediately when running tests, so I added a 5 second pause before running tests so that jekyll actually brings everything up as expected first

What's different:

  • _conferences/__main__.py got refactored!
    • Moved most top-level logic into callable (and testable) functions
    • Added a "main" function to the script that orders the steps taken
  • Introduced a delay between tests: playwright tests don't consistently work during CI runs. It seems like there is some internal setup/teardown that happens between tests that don't always complete in time for a test case to run. Adding a delay seems like an easy bandaid to slow things down enough to get them running more consistently

Those tests though:

I get all tests passing when I run the tests locally!
image

Other Notes:

I caught why the URL was always being changed to None as mentioned in #384, in the original logic I forgot to set the valid_url variable to a real value when there is a valid url found 😅
I was so focused on the unhappy path, that I neglected the happy one 😭

@dragid10
Copy link
Member Author

dragid10 commented Aug 31, 2024

Edit: After a good night's rest, I think I figured out why the CI was failing. The GITHUB_TOKEN not being available was just a red herring. I've added commits to the PR to hopefully address the problem

Hmm interestingly enough, I'm able to run the tests just fine locally. I'm not at all sure why the CI doesn't think it's getting a GITHUB_TOKEN

$ python -m pytest
==================================================================== test session starts =====================================================================
platform darwin -- Python 3.11.3, pytest-8.3.2, pluggy-1.5.0 -- /Users/alexoladele/Coding/side-projects/blackpythondevs.github.io/.venv/bin/python
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase(PosixPath('/Users/alexoladele/Coding/side-projects/blackpythondevs.github.io/.hypothesis/examples'))
rootdir: /Users/alexoladele/Coding/side-projects/blackpythondevs.github.io
configfile: pytest.ini
plugins: asyncio-0.23.8, playwright-0.5.1, hypothesis-6.111.1, base-url-2.1.0, pyodide-0.58.3
asyncio: mode=Mode.STRICT
collected 26 items                                                                                                                                           

tests/test.py::test_destination[chromium-about] PASSED                                                                                                 [  3%]
tests/test.py::test_destination[chromium-community] PASSED                                                                                             [  7%]
tests/test.py::test_destination[chromium-conferences] PASSED                                                                                           [ 11%]
tests/test.py::test_destination[chromium-events] PASSED                                                                                                [ 15%]
tests/test.py::test_headers_in_language[chromium-Acerca de-/es/about/] PASSED                                                                          [ 19%]
tests/test.py::test_headers_in_language[chromium-Inicio-/es/] PASSED                                                                                   [ 23%]
tests/test.py::test_headers_in_language[chromium-Eventos-/es/events/] PASSED                                                                           [ 26%]
tests/test.py::test_headers_in_language[chromium-Comunidad-/es/community/] PASSED                                                                      [ 30%]
tests/test.py::test_headers_in_language[chromium-Conferencias-/es/conferences/] PASSED                                                                 [ 34%]
tests/test.py::test_switching_lang_es_about[chromium] PASSED                                                                                           [ 38%]
tests/test.py::test_headers_in_sw[chromium-Kutuhusu-/sw/about/] PASSED                                                                                 [ 42%]
tests/test.py::test_headers_in_sw[chromium-Nyumbani-/sw/] PASSED                                                                                       [ 46%]
tests/test.py::test_headers_in_sw[chromium-Matukio-/sw/events/] PASSED                                                                                 [ 50%]
tests/test.py::test_headers_in_sw[chromium-Jumuiya-/sw/community/] PASSED                                                                              [ 53%]
tests/test.py::test_headers_in_sw[chromium-Mikutano-/sw/conferences/] PASSED                                                                           [ 57%]
tests/test.py::test_switching_lang_sw_about[chromium] PASSED                                                                                           [ 61%]
tests/test.py::test_bpdevs_title_en[chromium-Black Python Devs | Home-/] PASSED                                                                        [ 65%]
tests/test.py::test_bpdevs_title_en[chromium-Black Python Devs | Blog-/blog] PASSED                                                                    [ 69%]
tests/test.py::test_bpdevs_title_en[chromium-Black Python Devs | About Us-/about/] PASSED                                                              [ 73%]
tests/test.py::test_bpdevs_title_en[chromium-Black Python Devs | Events-/events/] PASSED                                                               [ 76%]
tests/test.py::test_bpdevs_title_en[chromium-Black Python Devs | Conferences-/conferences/] PASSED                                                     [ 80%]
tests/test.py::test_bpdevs_title_en[chromium-Black Python Devs | Community-/community/] PASSED                                                         [ 84%]
tests/test.py::test_mailto_bpdevs[chromium] PASSED                                                                                                     [ 88%]
tests/test.py::test_conference_parsing_valid_url PASSED                                                                                                [ 92%]
tests/test.py::test_conference_parsing_logic_no_url_scheme PASSED                                                                                      [ 96%]
tests/test.py::test_conference_parsing_logic_no_url PASSED     

@dragid10
Copy link
Member Author

@kjaymiller okay I promise I'm done making updates. This is forrealsies ready for review!

tests/test.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

How is the service started. I wonder if a fixture that starts the service and tears it down would be more reliable.

Copy link
Contributor

@kjaymiller kjaymiller left a comment

Choose a reason for hiding this comment

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

Great work

really minor change and a question. looks good though and about 99% there

@kjaymiller
Copy link
Contributor

@dragid10 - please run pre-commit run --all

@kjaymiller kjaymiller merged commit b6ce359 into BlackPythonDevs:gh-pages Sep 1, 2024
2 checks passed
@dragid10 dragid10 deleted the add-conf-parsing-test branch September 12, 2024 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

adds tests for Conferences script
2 participants