Skip to content

test: replace test_should_ naming with test_verb_noun_context convention #44

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

Closed
aivong-openhands wants to merge 17 commits into from
Closed

test: replace test_should_ naming with test_verb_noun_context convention #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9a7bf0b
test: replace test_should_ naming with test_verb_noun_context convention
Apr 17, 2026
3ee01b2
fix(lint): fix import order in naming convention tests for ruff v0.4.1
Apr 17, 2026
2068694
fix(enterprise): Apply deployment LLM proxy URL override in /api/v1/u…
xingyaoww Apr 17, 2026
5c8d7c4
Fix CVE-2026-40347: Update python-multipart to 0.0.26 (#13965)
aivong-openhands Apr 17, 2026
a5a7a86
chore(deps): bump actions/github-script from 7 to 9 (#13958)
dependabot[bot] Apr 17, 2026
5bb9e4a
docs: fix broken React Router test doc links (#13802)
buyua9 Apr 17, 2026
8f5b3ce
fix(settings): align MCP settings layout with other settings pages (#…
octo-patch Apr 18, 2026
9db83a1
Refresh git settings after provider updates (#13979)
neubig Apr 18, 2026
e23af62
PLTF-1269: add docstrings to mock classes in maintenance runner tests…
aivong-openhands Apr 18, 2026
b87f08f
docs: add note about updating enterprise poetry lockfile (#13761)
aivong-openhands Apr 18, 2026
1af04f2
docs: fix app_server README to reflect actual module structure (#13890)
JefferyLiu6 Apr 18, 2026
b841e1a
chore(deps): bump docker/metadata-action from 5 to 6 (#13959)
dependabot[bot] Apr 18, 2026
1fd9467
chore(deps): bump actions/download-artifact from 7 to 8 (#13700)
dependabot[bot] Apr 18, 2026
1651edf
chore(deps): bump docker/login-action from 3 to 4 (#13960)
dependabot[bot] Apr 18, 2026
e8249f0
docs: align AgentHub task tracking references (#13708)
xsfX20 Apr 18, 2026
0628679
chore(deps): bump docker/build-push-action from 6 to 7 (#13961)
dependabot[bot] Apr 18, 2026
988ec4a
Merge branch 'main' into test/fix-naming-convention
aivong-openhands Apr 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .github/workflows/ghcr-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ jobs:
packages: write
steps:
- name: Login to GHCR
uses: docker/login-action@v3
uses: docker/login-action@v4
with:
registry: ghcr.io
username: ${{ github.repository_owner }}
Expand Down Expand Up @@ -202,7 +202,7 @@ jobs:
echo "EOF" >> "$GITHUB_OUTPUT"
- name: Build and push runtime image ${{ matrix.base_image.image }}
if: github.event.pull_request.head.repo.fork != true
uses: docker/build-push-action@v6
uses: docker/build-push-action@v7
with:
push: true
tags: ${{ env.DOCKER_TAGS }}
Expand All @@ -215,7 +215,7 @@ jobs:
# Forked repos can't push to GHCR, so we just build in order to populate the cache for rebuilding
- name: Build runtime image ${{ matrix.base_image.image }} for fork
if: github.event.pull_request.head.repo.fork
uses: docker/build-push-action@v6
uses: docker/build-push-action@v7
with:
tags: ghcr.io/${{ env.REPO_OWNER }}/runtime:${{ env.RELEVANT_SHA }}-${{ matrix.base_image.tag }}
context: containers/runtime
Expand All @@ -236,7 +236,7 @@ jobs:
packages: write
steps:
- name: Login to GHCR
uses: docker/login-action@v3
uses: docker/login-action@v4
with:
registry: ghcr.io
username: ${{ github.repository_owner }}
Expand Down Expand Up @@ -324,7 +324,7 @@ jobs:
# Also compute base tags (no arch suffix) for the merge job output
- name: Extract base metadata for merge
id: meta_base
uses: docker/metadata-action@v5
uses: docker/metadata-action@v6
with:
images: ghcr.io/openhands/enterprise-server
tags: |
Expand All @@ -350,7 +350,7 @@ jobs:
# rather than a mutable branch tag like "main" which can serve stale cached layers.
echo "OPENHANDS_DOCKER_TAG=${RELEVANT_SHA}" >> $GITHUB_ENV
- name: Build and push Docker image
uses: docker/build-push-action@v6
uses: docker/build-push-action@v7
with:
context: .
file: enterprise/Dockerfile
Expand All @@ -375,7 +375,7 @@ jobs:
if: github.event.pull_request.head.repo.fork != true
steps:
- name: Login to GHCR
uses: docker/login-action@v3
uses: docker/login-action@v4
with:
registry: ghcr.io
username: ${{ github.repository_owner }}
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/openhands-resolver.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ jobs:
echo "TARGET_BRANCH=${{ inputs.target_branch || 'main' }}" >> $GITHUB_ENV

- name: Comment on issue with start message
uses: actions/github-script@v7
uses: actions/github-script@v9
with:
github-token: ${{ secrets.PAT_TOKEN || github.token }}
script: |
Expand All @@ -206,7 +206,7 @@ jobs:

- name: Install OpenHands
id: install_openhands
uses: actions/github-script@v7
uses: actions/github-script@v9
env:
COMMENT_BODY: ${{ github.event.comment.body || '' }}
REVIEW_BODY: ${{ github.event.review.body || '' }}
Expand Down Expand Up @@ -305,7 +305,7 @@ jobs:

# Step leaves comment for when agent is invoked on PR
- name: Analyze Push Logs (Updated PR or No Changes) # Skip comment if PR update was successful OR leave comment if the agent made no code changes
uses: actions/github-script@v7
uses: actions/github-script@v9
if: always()
env:
AGENT_RESPONDED: ${{ env.AGENT_RESPONDED || 'false' }}
Expand Down Expand Up @@ -341,7 +341,7 @@ jobs:

# Step leaves comment for when agent is invoked on issue
- name: Comment on issue # Comment link to either PR or branch created by agent
uses: actions/github-script@v7
uses: actions/github-script@v9
if: always() # Comment on issue even if the previous steps fail
env:
AGENT_RESPONDED: ${{ env.AGENT_RESPONDED || 'false' }}
Expand Down Expand Up @@ -416,7 +416,7 @@ jobs:

# Leave error comment when both PR/Issue comment handling fail
- name: Fallback Error Comment
uses: actions/github-script@v7
uses: actions/github-script@v9
if: ${{ env.AGENT_RESPONDED == 'false' }} # Only run if no conditions were met in previous steps
env:
ISSUE_NUMBER: ${{ env.ISSUE_NUMBER }}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pr-artifacts.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ jobs:

- name: Update PR comment after cleanup
if: steps.check-fork.outputs.is_fork == 'false' && steps.remove.outputs.removed == 'true'
uses: actions/github-script@v7
uses: actions/github-script@v9
with:
script: |
const marker = '<!-- pr-artifacts-notice -->';
Expand Down Expand Up @@ -107,7 +107,7 @@ jobs:

- name: Post or update PR comment
if: steps.check.outputs.exists == 'true'
uses: actions/github-script@v7
uses: actions/github-script@v9
with:
script: |
const marker = '<!-- pr-artifacts-notice -->';
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/py-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ jobs:
steps:
- uses: actions/checkout@v6

- uses: actions/download-artifact@v7
- uses: actions/download-artifact@v8
id: download
with:
pattern: coverage-*
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/welcome-good-first-issue.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
steps:
- name: Check if welcome comment already exists
id: check_comment
uses: actions/github-script@v7
uses: actions/github-script@v9
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 18, 2026

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the upgraded github-script blocks do not use v9-incompatible patterns.
# Expected: matches for actions/github-script@v9 are OK; no matches for require('@actions/github')
# or `const/let getOctokit` redeclarations should appear.
rg -n -C2 "actions/github-script@v9|require\\(['\"]@actions/github['\"]\\)|\\b(const|let)\\s+getOctokit\\b" .github/workflows/welcome-good-first-issue.yml .github/workflows/openhands-resolver.yml .github/workflows/pr-artifacts.yml

Repository: aivong-openhands/OpenHands

Length of output: 4048

🏁 Script executed:

#!/bin/bash
# Check git diff to see what changed in workflow files
git diff HEAD -- .github/workflows/welcome-good-first-issue.yml .github/workflows/openhands-resolver.yml .github/workflows/pr-artifacts.yml | head -200

Repository: aivong-openhands/OpenHands

Length of output: 52

🏁 Script executed:

#!/bin/bash
# Get full content of the changed workflow file to understand context
cat -n .github/workflows/welcome-good-first-issue.yml

Repository: aivong-openhands/OpenHands

Length of output: 2759

🏁 Script executed:

#!/bin/bash
# Check git log for recent changes to the workflow files
git log --oneline -10 -- .github/workflows/welcome-good-first-issue.yml .github/workflows/openhands-resolver.yml .github/workflows/pr-artifacts.yml

Repository: aivong-openhands/OpenHands

Length of output: 130

🏁 Script executed:

#!/bin/bash
# Check what version was in the previous commit
git show HEAD~1:.github/workflows/welcome-good-first-issue.yml 2>/dev/null | grep -n "actions/github-script" || echo "File or pattern not found in previous commit"

Repository: aivong-openhands/OpenHands

Length of output: 114

🏁 Script executed:

#!/bin/bash
# Check git status to see which files are staged/modified
git status --short

Repository: aivong-openhands/OpenHands

Length of output: 134

🏁 Script executed:

#!/bin/bash
# Check what's in HEAD for the workflow file
git show HEAD:.github/workflows/welcome-good-first-issue.yml | grep -n "actions/github-script"

Repository: aivong-openhands/OpenHands

Length of output: 153

🏁 Script executed:

#!/bin/bash
# Check parent commit
git log --oneline -3 -- .github/workflows/welcome-good-first-issue.yml

Repository: aivong-openhands/OpenHands

Length of output: 130

🏁 Script executed:

#!/bin/bash
# Check if the file was modified in any of the recent commits
git show HEAD -- .github/workflows/welcome-good-first-issue.yml | head -50

Repository: aivong-openhands/OpenHands

Length of output: 1569

🏁 Script executed:

#!/bin/bash
# Check what files were actually changed in this PR/branch
git diff main...HEAD --name-status

Repository: aivong-openhands/OpenHands

Length of output: 105

🏁 Script executed:

#!/bin/bash
# Get a summary of the PR changes
git log --oneline main..HEAD | head -20

Repository: aivong-openhands/OpenHands

Length of output: 130

🏁 Script executed:

#!/bin/bash
# Check the branches and their relationship
git branch -a

Repository: aivong-openhands/OpenHands

Length of output: 167

🏁 Script executed:

#!/bin/bash
# Check what changed in the merge commit specifically
git show 988ec4aa --stat

Repository: aivong-openhands/OpenHands

Length of output: 50383

🏁 Script executed:

#!/bin/bash
# Look at the full diff of the merge commit
git show 988ec4aa | head -300

Repository: aivong-openhands/OpenHands

Length of output: 14238

🏁 Script executed:

#!/bin/bash
# Check which workflow files were actually modified/added in this PR
git diff --name-status HEAD~1 HEAD -- .github/workflows/ 2>/dev/null || echo "No diff available with HEAD~1"

# Check all workflow files added/modified compared to main
git diff --name-status origin/main...HEAD -- .github/workflows/ 2>/dev/null || echo "Cannot compare to origin/main"

Repository: aivong-openhands/OpenHands

Length of output: 129

🏁 Script executed:

#!/bin/bash
# Check if openhands-resolver.yml and pr-artifacts.yml were modified
git log -p --follow -1 -- .github/workflows/openhands-resolver.yml | head -50

Repository: aivong-openhands/OpenHands

Length of output: 52

🏁 Script executed:

#!/bin/bash
# Get the commit message to understand PR scope
git log --format=%B -1 988ec4aa

Repository: aivong-openhands/OpenHands

Length of output: 121

🏁 Script executed:

#!/bin/bash
# Check if welcome-good-first-issue.yml existed in origin/main
git show origin/main:.github/workflows/welcome-good-first-issue.yml 2>&1 | head -20

Repository: aivong-openhands/OpenHands

Length of output: 483

🏁 Script executed:

#!/bin/bash
# List all files in the current branch's .github/workflows
ls -la .github/workflows/ | grep -E "welcome|good"

Repository: aivong-openhands/OpenHands

Length of output: 148

🏁 Script executed:

#!/bin/bash
# Check version in origin/main for the other workflow files
echo "=== openhands-resolver.yml in origin/main ==="
git show origin/main:.github/workflows/openhands-resolver.yml 2>/dev/null | grep -n "actions/github-script" | head -5

echo "=== pr-artifacts.yml in origin/main ==="
git show origin/main:.github/workflows/pr-artifacts.yml 2>/dev/null | grep -n "actions/github-script" | head -5

Repository: aivong-openhands/OpenHands

Length of output: 467

🏁 Script executed:

#!/bin/bash
# Get the actual changes to welcome-good-first-issue.yml
git diff origin/main...HEAD -- .github/workflows/welcome-good-first-issue.yml | head -100

Repository: aivong-openhands/OpenHands

Length of output: 110

🏁 Script executed:

#!/bin/bash
# Check the PR description or branch info to understand scope
git log --all --format=%B --grep="test" --grep="naming" -i | head -50

Repository: aivong-openhands/OpenHands

Length of output: 1819

Split this major workflow upgrade from the test-name refactor.

The stated PR scope indicates test-name changes, but this PR upgrades actions/github-script from v7 to v9 across three workflow files (9 total uses). This is a major version bump with breaking changes documented in the v9.0.0 release notes. While the embedded scripts in these workflows are compatible with v9 (using only injected github and context objects), the dependency upgrade should either be a separate PR or the PR description/testing plan should explicitly cover workflow validation for this change.

Also applies to: lines 36, .github/workflows/openhands-resolver.yml (lines 195, 209, 308, 344, 419), .github/workflows/pr-artifacts.yml (lines 62, 110)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/welcome-good-first-issue.yml at line 17, The PR mixes a
test-name refactor with a breaking dependency bump: multiple workflow steps
using the action identifier "uses: actions/github-script@v9" were introduced
here; either revert those steps back to the prior pin (e.g.,
actions/github-script@v7) in this PR or move all occurrences of "uses:
actions/github-script@v9" into a separate PR that upgrades the action, and
update the PR description to explicitly state the workflow upgrade and include a
validation plan (manual workflow run or CI workflow tests) verifying the
embedded scripts still work with v9 before landing.

with:
result-encoding: string
script: |
Expand All @@ -33,7 +33,7 @@ jobs:

- name: Leave welcome comment
if: steps.check_comment.outputs.result == 'false'
uses: actions/github-script@v7
uses: actions/github-script@v9
with:
script: |
const repoUrl = `https://github.com/${context.repo.owner}/${context.repo.repo}`;
Expand Down
1 change: 1 addition & 0 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ Each integration follows a consistent pattern with service classes, storage mode
- Database changes require careful migration planning in `enterprise/migrations/`
- Always test changes in both OpenHands and enterprise contexts
- Use the enterprise-specific Makefile commands for development
- When the `openhands-ai` package (root project) version has been updated, run `poetry lock` in the `enterprise/` folder to update the version in the enterprise poetry lockfile.

**Enterprise Testing Best Practices:**

Expand Down
6 changes: 3 additions & 3 deletions enterprise/poetry.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 7 additions & 3 deletions enterprise/server/routes/users_v1.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
from server.auth.saas_user_auth import SaasUserAuth
from server.models.user_models import SaasUserInfo

from openhands.app_server.config import depends_user_context
from openhands.app_server.config import (
depends_user_context,
resolve_provider_llm_base_url,
)
from openhands.app_server.sandbox.session_auth import validate_session_key_ownership
from openhands.app_server.user.auth_user_context import AuthUserContext
from openhands.app_server.user.user_context import UserContext
Expand Down Expand Up @@ -42,8 +45,9 @@ def _inject_sdk_compat_fields(
"""
agent_settings = content.get('agent_settings') or {}
llm = agent_settings.get('llm') or {}
content['llm_model'] = llm.get('model')
content['llm_base_url'] = llm.get('base_url')
model = llm.get('model')
content['llm_model'] = model
content['llm_base_url'] = resolve_provider_llm_base_url(model, llm.get('base_url'))
if include_api_key:
content['llm_api_key'] = llm.get('api_key')
content['mcp_config'] = agent_settings.get('mcp_config')
Expand Down
20 changes: 10 additions & 10 deletions enterprise/tests/unit/test_auth_routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1281,7 +1281,7 @@ class TestKeycloakCallbackRecaptcha:
"""Tests for reCAPTCHA integration in keycloak_callback()."""

@pytest.mark.asyncio
async def test_should_verify_recaptcha_and_allow_login_when_score_is_high(
async def test_login_allows_when_recaptcha_score_is_high(
self, mock_request, create_keycloak_user_info
):
"""Test that login proceeds when reCAPTCHA score is high."""
Expand Down Expand Up @@ -1369,7 +1369,7 @@ async def test_should_verify_recaptcha_and_allow_login_when_score_is_high(
mock_recaptcha_service.create_assessment.assert_called_once()

@pytest.mark.asyncio
async def test_should_block_login_when_recaptcha_score_is_low(
async def test_login_blocks_when_recaptcha_score_is_low(
self, mock_request, create_keycloak_user_info
):
"""Test that login is blocked and redirected when reCAPTCHA score is low."""
Expand Down Expand Up @@ -1439,7 +1439,7 @@ async def test_should_block_login_when_recaptcha_score_is_low(
assert 'recaptcha_blocked=true' in result.headers['location']

@pytest.mark.asyncio
async def test_should_extract_ip_from_x_forwarded_for_header(
async def test_login_extracts_ip_from_x_forwarded_for(
self, mock_request, create_keycloak_user_info
):
"""Test that IP is extracted from X-Forwarded-For header when present."""
Expand Down Expand Up @@ -1528,7 +1528,7 @@ async def test_should_extract_ip_from_x_forwarded_for_header(
assert call_args[1]['user_ip'] == '192.168.1.1'

@pytest.mark.asyncio
async def test_should_use_client_host_when_x_forwarded_for_missing(
async def test_login_uses_client_host_when_x_forwarded_for_missing(
self, mock_request, create_keycloak_user_info
):
"""Test that client.host is used when X-Forwarded-For is missing."""
Expand Down Expand Up @@ -1618,7 +1618,7 @@ async def test_should_use_client_host_when_x_forwarded_for_missing(
assert call_args[1]['user_ip'] == '192.168.1.2'

@pytest.mark.asyncio
async def test_should_use_unknown_ip_when_client_is_none(
async def test_login_uses_unknown_ip_when_client_is_none(
self, mock_request, create_keycloak_user_info
):
"""Test that 'unknown' IP is used when client is None."""
Expand Down Expand Up @@ -1707,7 +1707,7 @@ async def test_should_use_unknown_ip_when_client_is_none(
assert call_args[1]['user_ip'] == 'unknown'

@pytest.mark.asyncio
async def test_should_include_email_in_assessment_when_available(
async def test_login_includes_email_in_assessment(
self, mock_request, create_keycloak_user_info
):
"""Test that email is included in assessment when available."""
Expand Down Expand Up @@ -1793,7 +1793,7 @@ async def test_should_include_email_in_assessment_when_available(
assert call_args[1]['email'] == 'user@example.com'

@pytest.mark.asyncio
async def test_should_skip_recaptcha_when_site_key_not_configured(
async def test_login_skips_recaptcha_when_site_key_not_configured(
self, mock_request, create_keycloak_user_info
):
"""Test that reCAPTCHA is skipped when RECAPTCHA_SITE_KEY is not configured."""
Expand Down Expand Up @@ -1870,7 +1870,7 @@ async def test_should_skip_recaptcha_when_site_key_not_configured(
mock_recaptcha_service.create_assessment.assert_not_called()

@pytest.mark.asyncio
async def test_should_skip_recaptcha_when_token_is_missing(
async def test_login_skips_recaptcha_when_token_is_missing(
self, mock_request, create_keycloak_user_info
):
"""Test that reCAPTCHA is skipped when token is missing from state."""
Expand Down Expand Up @@ -1941,7 +1941,7 @@ async def test_should_skip_recaptcha_when_token_is_missing(
mock_recaptcha_service.create_assessment.assert_not_called()

@pytest.mark.asyncio
async def test_should_fail_open_when_recaptcha_service_throws_exception(
async def test_login_fails_open_when_recaptcha_raises(
self, mock_request, create_keycloak_user_info
):
"""Test that login proceeds (fail open) when reCAPTCHA service throws exception."""
Expand Down Expand Up @@ -2029,7 +2029,7 @@ async def test_should_fail_open_when_recaptcha_service_throws_exception(
assert len(recaptcha_error_calls) > 0

@pytest.mark.asyncio
async def test_should_log_warning_when_recaptcha_blocks_user(
async def test_login_logs_warning_when_recaptcha_blocks(
self, mock_request, create_keycloak_user_info
):
"""Test that warning is logged when reCAPTCHA blocks user."""
Expand Down
Loading
Loading