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

Add tests to ensure workflow doesn't get broken #7631

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add tests to ensure workflow doesn't get broken #7631

wants to merge 1 commit into from
+876 −0

Conversation

erkinalp
Copy link

@erkinalp erkinalp commented Apr 1, 2025 •
edited by mamoodi
Loading

  • This change is worth documenting at https://docs.all-hands.dev/
  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

End-user friendly description of the problem this fixes or functionality that this introduces.
Added comprehensive test suite for workflow token authentication to ensure that token handling in GitHub and GitLab workflows remains functional and secure.

Give a summary of what the PR does, explaining any non-trivial design decisions.
This PR implements tests for workflow token authentication as requested in issue #7573. It includes:

  1. Unit tests for token handling in GitHub workflows

    • Tests for GitHub token validation
    • Tests for GitHub PAT token handling
    • Tests for GitLab token authentication
    • Tests for missing token scenarios

  2. Local workflow testing using the act tool

    • Simulated workflow executions with different token types
    • Verification of token validation in workflow files

  3. Integration tests for token authentication

    • End-to-end testing of token handling in workflows
    • Tests for token permissions and security constraints

The tests use mocking to avoid requiring actual API calls while still providing comprehensive coverage of token handling functionality.

Link of any specific issues this addresses.
Fixes #7573

Link to Devin run: https://app.devin.ai/sessions/9e4698444aa147bf8d970a4faec2de3e

Co-authored-by: Erkin Alp Güney [email protected]

@devin-ai-integration @erkinalp
- Added unit tests for token handling in GitHub workflows
c079158 
- Implemented mock-based API tests for GitHub/GitLab interactions
- Added local workflow testing with act
- Created integration tests for token authentication
- Tests cover GitHub token, GitHub PAT, and GitLab token types

Co-Authored-By: Erkin Alp Güney <[email protected]>
@erkinalp erkinalp changed the title Add tests to ensure workflow doesn't get broken (#7573) Add tests to ensure workflow doesn't get broken Apr 1, 2025
neubig
neubig requested changes Apr 7, 2025
View reviewed changes
Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

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

Thanks @erkinalp ! Could you please confirm that this PR actually fixes issue #7573? One way to do so would be to run these tests locally and make sure that they pass with the current code, and fail when there is a small mistake in the current code corresponding to the issues raised in issue #7573. If you can do this, could you attach screenshots of each so I know it's properly tested?

@erkinalp
Copy link
Author

erkinalp commented Apr 8, 2025

I'll soon deal with it when I have time.

@tawago
Copy link
Contributor

tawago commented Apr 9, 2025 •
edited
Loading

FYI: (because it's lacking --selected_repo arg,) send_pull_request.py fails in the current openhands-resolver.yml if you use github_token generated from the workflow. Does this PR catch it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neubig neubig neubig requested changes

Requested changes must be addressed to merge this pull request.

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

Successfully merging this pull request may close these issues.

Add tests to ensure workflow doesn't get broken
3 participants
@erkinalp @tawago @neubig