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.

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

[4/?] Add ability to run code examples in the playground: Get pony snippets tested with ponyc at CI time #550

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

shaedrich
Copy link
Collaborator

Fix #549

See #340

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label May 31, 2024
Copy link

netlify bot commented May 31, 2024

Deploy Preview for pony-tutorial ready!

Name Link
🔨 Latest commit 0eff5c9
🔍 Latest deploy log https://app.netlify.com/sites/pony-tutorial/deploys/66592377f44a040008b80541
😎 Deploy Preview https://deploy-preview-550--pony-tutorial.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@shaedrich
Copy link
Collaborator Author

fyi: The new job fails because currently only one code snippet has expectations set (all of them have to)

Comment on lines +43 to +46
response=$(curl -s --header "Content-Type: application/json" \
--request POST \
--data "$json" \
"https://playground.ponylang.io/evaluate.json")
Copy link
Member

Choose a reason for hiding this comment

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

Rather than calling out to the playground to run these, we should use ponyc directly here. We don't want to flood the playground when running these tests, and we don't want the uptime of the playground to affect these tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, I implemented it in a way I'm familiar, since nobody replied to "I still need to figure out, how that might work 🤔 If there's someone else who would take this on, I wouldn't be mad 😅" in #340. I can try doing it via ponyc but how it will turn out is up to be shown.

},
"appendices-examples-access-command-line-arguments.pony": {
"runnable": true,
"stdout": "This is printed first\nThis is printed last\n",
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't need to test stdout of running the examples - we just need to prove that an example successfully compiles (or fails to compile with a particular message in stderr). We don't need to run the examples at all - just compile them.

Copy link
Collaborator Author

@shaedrich shaedrich Jun 4, 2024

Choose a reason for hiding this comment

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

What's the point of a snippet if it does something but we don't even know if it illustrates what the chapter talks about? It just happens to have a few buzzwords in it. Seeing them compile successfully is nice but very surface-level to me. There actually have been examples that failed running in the playground despite it successfully running was to be expected because nobody bothered to check it in the playground before. Nice letting users of the tutorial twist in the wind with compilable but unrunnable code—yay! 🎉

@SeanTAllen
Copy link
Member

SeanTAllen commented Jun 4, 2024

For things that are "ci support scripts" our pattern is to put in .ci-scripts at the top level. If we have an external script like you currently have, it should go there, not in workflows.

@SeanTAllen
Copy link
Member

When we add this, we should have a job that is triggered by the new release of a nightly image and new release of a release image to verify we dont have breakage. We have examples of this in a variety of library files. Once this basic on PR change is in, someone ping me and i'll add the "on update" workflows.

@shaedrich
Copy link
Collaborator Author

When we add this, we should have a job that is triggered by the new release of a nightly image and new release of a release image to verify we dont have breakage. We have examples of this in a variety of library files. Once this basic on PR change is in, someone ping me and i'll add the "on update" workflows.

@SeanTAllen As explained above (#550 (comment)) to @jemc, the focus of this PR is not my strong suit and it would be great if someone else could tackle this

After all, I guess, [4/?] should start from scratch with a new PR as the implementation will vastly differ from what I tried.

@jemc
Copy link
Member

jemc commented Jun 4, 2024

That's okay if you don't want to continue on this particular sub-task - we can try to find someone to pick up this work once your two remaining prior sub-tasks (i.e #545 , #542) are merged.

@shaedrich
Copy link
Collaborator Author

shaedrich commented Jun 4, 2024

#550 is not dependent on #542 so they can be worked on in parallel if wanted.

@shaedrich
Copy link
Collaborator Author

Okay, I guess, I managed to do an implementation with #551, which indeed has to wait for #545 to throw some less errors

@jemc jemc removed the discuss during sync Should be discussed during an upcoming sync label Jun 25, 2024
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.

[4/?] Add ability to run code examples in the playground: Get pony snippets tested with ponyc at CI time
4 participants