Skip to content

Conversation

Activ8-AI
Copy link

…adge

Description

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Testing

  • Tests pass locally (go test -v ./...)
  • Added/updated tests for new functionality

Checklist

  • Code follows project style guidelines
  • Self-reviewed the code
  • Added necessary documentation
  • No new warnings or errors

@Copilot Copilot AI review requested due to automatic review settings September 22, 2025 11:30
@Activ8-AI Activ8-AI requested a review from a team as a code owner September 22, 2025 11:30
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive testing infrastructure to the project, including a smoke test script for end-to-end validation, build automation, and continuous integration. The smoke test validates the HTTP server startup, health endpoint, and CLI functionality.

  • Adds a smoke test script that validates server startup and basic functionality
  • Integrates smoke testing into the build system via Makefile
  • Sets up GitHub Actions CI workflow for automated testing
  • Adds CI status badge to README

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
scripts/smoke.sh New smoke test script that starts the HTTP server and validates basic functionality
README.md Adds CI status badge
Makefile Adds smoke test target
.github/workflows/ci.yml New CI workflow with build, unit tests, and smoke tests

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


if ! lsof -ti tcp:"${PORT}" >/dev/null 2>&1; then
echo "[smoke] server failed to start; logs:" >&2
sed -n '1,200p' "${LOGFILE}" >&2 || true
Copy link
Preview

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

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

[nitpick] The magic number '200' is duplicated. Consider defining a constant like MAX_LOG_LINES=200 at the top of the script to improve maintainability.

Copilot uses AI. Check for mistakes.

@rafaeljusto
Copy link
Contributor

Hi, thanks for the contribution! From what I can see, this only starts the MCP HTTP server and checks if the health endpoint is running. 🤔

Could we add it as part of the existing test suite?

test:
runs-on: ubuntu-latest
needs: build
steps:
- uses: actions/checkout@v5
with:
persist-credentials: false
- name: Set up Go
uses: actions/setup-go@v6
with:
go-version: ${{ env.GO_VERSION }}
- name: Download modules
run: go mod download
- name: Test
run: go test -v ./...

A new cmd/mcp-http/main_test.go could contain the test code to start the server and do the basic checks, which could include health and tools endpoints. This could even allow using the MCP client library to improve response checks. The change would save us from extra GHA workflows and scripts.

@Activ8-AI Activ8-AI closed this Sep 29, 2025
@Activ8-AI Activ8-AI reopened this Sep 29, 2025
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.

2 participants