BlackPythonDevs
adds tests for Conferences script
#379 was a good add but I'm noticing the change has set the values to null in #381.
This leads me to believe there is a bug in there somewhere that needs to be addressed. Can we create a test that validates this conferences script.
This can be added to our ci.
- [ ] #385
@dragid10 - Did you want to take a look at this since you implemented the original fix?
@dragid10 not intending to rush you on this. Simply checking in on if any assistance was needed in figuring this out?
Apologies, I haven't had a lot of bandwidth this past week to work on this. But I should have some bandwidth later today and tomorrow morning!
@kjaymiller if we're looking to test the conference parsing logic, I think it'd make sense to refactor parts of _conferences/_main_.py to be callable functions as opposed to having the logic be top-level.
I think doing this refactor would allow us to pass in dummy values to a function when testing, instead of always trying to get the current open issues in the repo.
Here's a loose example of what I'm thinking about:
from _conferences.__main__ import parse_conference_details
def test_conference_parsing_logic():
test_issue: list[dict] = [{"body": "xyz", "title": "[conference] test title", ...}]
# Function signature
# parse_conference_details(issues: list[Issue]) -> list[dict]
parsed_conferences = parse_conference_details(issues=test_issues)
...
assert parsed_conferences[0]["name"] == "xyz"
I think one of the downsides of the parse_conference_details signature in the above example is that it makes the function dependent on the github.Issue.Issue PyGitHub object. Meaning that we'd likely have to mock that object in the test
Alternatively, we could just decouple the actual parsing logic from the github.Issue.Issue object. It could look something like this:
for issue in open_issues:
if "conference" in [label.name for label in issue.labels]:
# Function signature
# parse_conference_details(issue_body: str) -> dict
parsed_conference = parse_conference_details(issue_body=issue.body)
...
These are my initial thoughts, let me know if I'm drastically overthinking this!
Nope I think you're asking the right questions! This all sounds good and I think the decoupling would be fine here.\