Skip to content

test: add unit tests for github + fix small handler bug#3280

Merged
anakin87 merged 2 commits into
mainfrom
github-tests
May 13, 2026
Merged

test: add unit tests for github + fix small handler bug#3280
anakin87 merged 2 commits into
mainfrom
github-tests

Conversation

@anakin87
Copy link
Copy Markdown
Member

@anakin87 anakin87 commented May 6, 2026 •
edited
Loading

Proposed Changes:

  • add more unit tests for GitHub integration, focusing on uncovered code paths (total coverage was below our 90% standard)

How did you test it?

CI

Checklist

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026 •
edited
Loading

Coverage report (github)

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)		Lines missing
  integrations/github/src/haystack_integrations/components/connectors/github
  file_editor.py
  pr_creator.py
  repo_forker.py
  repo_viewer.py
  integrations/github/src/haystack_integrations/tools/github
  utils.py
Project Total  

This report was generated by python-coverage-comment-action

@anakin87 anakin87 marked this pull request as ready for review May 6, 2026 12:28
@anakin87 anakin87 requested a review from a team as a code owner May 6, 2026 12:28
@anakin87 anakin87 requested review from bogdankostic and removed request for a team May 6, 2026 12:28
@anakin87 anakin87 self-assigned this May 6, 2026
bogdankostic
bogdankostic requested changes May 11, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@bogdankostic bogdankostic left a comment

Choose a reason for hiding this comment

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

Thanks, looking good in general, I just think that one of the test cases reveals a minor bug.

Comment thread integrations/github/tests/test_utils.py Outdated
Document(content="docs", meta={"type": "dir"}),
Document(content="README.md", meta={"type": "file"}),
],
"docsREADME.md",
Copy link
Copy Markdown
Contributor

@bogdankostic bogdankostic May 11, 2026

Choose a reason for hiding this comment

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

This expected output doesn't look right to me, we should probably adapt this line to

            result_str += (document.content or "") + "\n"

Copy link
Copy Markdown
Member Author

@anakin87 anakin87 May 13, 2026

Choose a reason for hiding this comment

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

oh, good catch. Fixed in dbeba70

@github-actions github-actions Bot added the type:documentation Improvements or additions to documentation label May 13, 2026
@anakin87 anakin87 requested a review from bogdankostic May 13, 2026 08:02
@anakin87 anakin87 changed the title test: add unit tests for github test: add unit tests for github + fix small handler bug May 13, 2026
@anakin87 anakin87 merged commit 30712d7 into main May 13, 2026
16 checks passed
@anakin87 anakin87 deleted the github-tests branch May 13, 2026 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bogdankostic bogdankostic bogdankostic approved these changes

Assignees

@anakin87 anakin87

Labels

integration:github type:documentation Improvements or additions to documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@anakin87 @bogdankostic