Skip to content

feat(ci): testing tuto capabilities in github actions#121

Open
farah-t-trigui wants to merge 2 commits intomainfrom
feat/automating-tuto-examples-testing
Open

feat(ci): testing tuto capabilities in github actions#121
farah-t-trigui wants to merge 2 commits intomainfrom
feat/automating-tuto-examples-testing

Conversation

@farah-t-trigui
Copy link
Contributor

A Github Actions workflow dedicated to testing the examples from the tutorial. The goal is to validate that the workflow behaves as expected when following the documented steps and sample configurations in the Wiki.

@farah-t-trigui farah-t-trigui force-pushed the feat/automating-tuto-examples-testing branch 2 times, most recently from 6ea6d4a to c7d1a89 Compare March 10, 2026 10:34
Copy link
Contributor

@eskenazit eskenazit left a comment

Choose a reason for hiding this comment

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

see the requested change to be implemented for all steps

echo "Step 1 endpoint ready!"
echo "Response: $RESPONSE"
exit 0
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

If we enter this else case, it means there was an error, so we should probably echo the (likely) invalid response and exit with 1

@farah-t-trigui farah-t-trigui force-pushed the feat/automating-tuto-examples-testing branch 2 times, most recently from 243e31a to e516b02 Compare March 10, 2026 11:22
@farah-t-trigui farah-t-trigui force-pushed the feat/automating-tuto-examples-testing branch 12 times, most recently from 667bf69 to 78388c4 Compare March 10, 2026 12:32

- name: Wait for /hello endpoint - My First Capability
run: |
sleep 2
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add comment explaining we need to wait for the framework to start. Also I ma wondering : should not we put the sleep at the end of the Run action rather than the wait one. What do you think ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keeping it in the wait step is better because the run step has one job which is to start the container. The wait step has one job: ensure it's ready

Copy link
Contributor

Choose a reason for hiding this comment

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

It means if we setup the run once but perform 70 tests on it, we will wait 70 times for 2 seconds ? If we know we need 2 seconds for the docker to be up and running, it feels more efficient to sleep when we setup it rather than every time we will call it, don't you think ?

ghcr.io/naftiko/framework:latest \
/app/step-1-my-first-capability.yml

- name: Wait for /hello endpoint - My First Capability
Copy link
Contributor

Choose a reason for hiding this comment

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

"Wait" does not fit semantically speaking, it is more "call" or "consume" the endpoint . Please fix in all related actions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, will do

- name: Show logs MCP Server
if: always()
run: docker logs mcp_server

Copy link
Contributor

Choose a reason for hiding this comment

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

We are missing an actual test here of the tools here. A list "tools/list" seems to be sufficient for now, please add it =)

@farah-t-trigui farah-t-trigui force-pushed the feat/automating-tuto-examples-testing branch 2 times, most recently from 6b48eda to d3129d7 Compare March 10, 2026 13:25
@farah-t-trigui farah-t-trigui force-pushed the feat/automating-tuto-examples-testing branch from d3129d7 to a606d79 Compare March 10, 2026 13:27
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