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

tests(ci): add basic CI tests and fix POST requests bug #68

Merged
merged 31 commits into from
Nov 8, 2023
Merged

tests(ci): add basic CI tests and fix POST requests bug #68

merged 31 commits into from
Nov 8, 2023

Conversation

Shillaker
Copy link
Contributor

@Shillaker Shillaker commented Oct 31, 2023 •
edited
Loading

Summary

What's changed?

  • Split CI build step (running on 16.x and 18.x) into separate workflow
  • Add CI step to test all examples on 16 and 18. Successful run here
  • Add CI unit test step (running on 18.x only;16/18 compatibility tested in examples). Successful run here
  • Add simple GET/POST request unit tests
  • Do not parse JSON inputs as an object (fix for bug(tests): local test is broken #64)
  • Add dev docs to help set up project locally
  • Split HTTP GET and POST examples

Why do we need this?

  • Currently no CI tests
  • Currently not checking examples work
  • The Scaleway function Node runtime does not take parsed JSON inputs (and currently POST requests break)

How have you tested it?

New CI tests.

Checklist

  • I have reviewed this myself
  • There is a unit test covering every change in this PR
  • I have updated the relevant documentation

@Shillaker Shillaker self-assigned this Oct 31, 2023
@Shillaker Shillaker changed the title fix(offline): fix POST requests issue tests(ci): add basic CI tests and fix POST requests bug Nov 6, 2023
@Shillaker Shillaker marked this pull request as ready for review November 6, 2023 18:05
Bemilie
Bemilie previously approved these changes Nov 7, 2023
View reviewed changes
Copy link
Collaborator

@Bemilie Bemilie left a comment

Choose a reason for hiding this comment

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

It looks greats, thanks for adding the tests with the GHA 👌

.github/workflows/tests.yml Show resolved Hide resolved
cyclimse
cyclimse previously approved these changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@cyclimse cyclimse left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, just a few nits. Don't forget to bump the version 😄! IMO, this can be a minor semantic version bump since it's a major fix to the framework.

package.json Show resolved Hide resolved
tests/requests.test.ts Outdated Show resolved Hide resolved
tests/requests.test.ts Outdated Show resolved Hide resolved
examples/with_http_get/handler.js Show resolved Hide resolved
@Shillaker Shillaker dismissed stale reviews from cyclimse and Bemilie via 5295fb3 November 7, 2023 13:35
cyclimse
cyclimse approved these changes Nov 8, 2023
View reviewed changes
Copy link
Contributor

@cyclimse cyclimse left a comment

Choose a reason for hiding this comment

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

Great job for the pipeline to run the examples!

@Shillaker Shillaker merged commit 514a1c1 into main Nov 8, 2023
23 checks passed
@Shillaker Shillaker deleted the post-fix branch November 8, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyclimse cyclimse cyclimse approved these changes

@Bemilie Bemilie Bemilie approved these changes

Assignees

@Shillaker Shillaker

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Shillaker @Bemilie @cyclimse