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

chore: add e2e testing #293

Merged
merged 21 commits into from
May 13, 2022
Merged

chore: add e2e testing #293

merged 21 commits into from
May 13, 2022

Conversation

mishabruml
Copy link
Contributor

@mishabruml mishabruml commented Mar 12, 2022

Resolves #281

Makefile in e2e test folder controls and runs the tests
Runs against projects in examples
For each project, builds and installs, (local version of serverless-esbuild used)
Runs serverless package and compares resulting files against a snapshot

@mishabruml mishabruml changed the title feat: first pass at e2e testing feat: first pass at e2e testing #281 Mar 12, 2022
@mishabruml mishabruml mentioned this pull request Mar 12, 2022
@mishabruml
Copy link
Contributor Author

hey @samchungy I'm having trouble to get this test flow to work in github actions. Locally it works fine. Would you mind trying it out locally on your machine? Any help appreciated 😄

@mishabruml
Copy link
Contributor Author

mishabruml commented Mar 15, 2022

Ok @samchungy I got this working, turns out it was inconsistent behaviour with cp so I've used rsync instead. Now all I need to do is use the branch version of the plugin to do the packaging rather than the version pulled from npm

@samchungy
Copy link
Collaborator

Keep tinkering away for now, I'll have to take a look over the weekend 😄 . This week I'm quite busy

@mishabruml
Copy link
Contributor Author

@samchungy this is now working as desired, ready for 👀 please 🙏 😀

samchungy
samchungy previously approved these changes Mar 22, 2022
Copy link
Collaborator

@samchungy samchungy left a comment

Choose a reason for hiding this comment

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

This is freaking awesome, I think it's a good base to work from. Thank you so much for your PR 🙇‍♂️ 🙏 . I think it should be good enough but I'm gonna wait for the blessing from the big man first though @floydspace. It's 1am for me too right now 🤣 so I'm gonna re-check this tomorrow some time.

@samchungy samchungy requested a review from floydspace March 22, 2022 14:17
@samchungy samchungy changed the title feat: first pass at e2e testing #281 chore: add e2e testing Mar 22, 2022
@mishabruml
Copy link
Contributor Author

This is freaking awesome, I think it's a good base to work from. Thank you so much for your PR 🙇‍♂️ 🙏 . I think it should be good enough but I'm gonna wait for the blessing from the big man first though @floydspace. It's 1am for me too right now 🤣 so I'm gonna re-check this tomorrow some time.

Thanks for your kind words @samchungy, you're welcome and I'm glad to be helping out 😄

@floydspace
Copy link
Owner

@mishabruml @samchungy Hey guys, I see some movements here, happy to see it. Curious to run it locally as well, give me some time, please.

@mishabruml
Copy link
Contributor Author

@floydspace npm run test:e2e or make -f e2e/Makefile to run them all, or make -f e2e/Makefile test-e2e-minimal to run e.g. just the test against examples/minimal 😄

@samchungy
Copy link
Collaborator

Perhaps it might be worth checking the output CF file too? Seeing as we mess with setting the handler for things.

@mishabruml
Copy link
Contributor Author

Perhaps it might be worth checking the output CF file too? Seeing as we mess with setting the handler for things.

Good idea, but a tricky problems with that is the file contains non-idempotent data which would break the snapshots (e.g. date/time stamps)

You may be able to parse the JSON in to an object and assert on specific nodes of the object or fuzzy match where needed

@samchungy
Copy link
Collaborator

Perhaps it might be worth checking the output CF file too? Seeing as we mess with setting the handler for things.

Good idea, but a tricky problems with that is the file contains non-idempotent data which would break the snapshots (e.g. date/time stamps)

You may be able to parse the JSON in to an object and assert on specific nodes of the object or fuzzy match where needed

Oooo yes I see what you mean. I don't mind what you've done with it though. 🎉

@mishabruml
Copy link
Contributor Author

Perhaps it might be worth checking the output CF file too? Seeing as we mess with setting the handler for things.

Good idea, but a tricky problems with that is the file contains non-idempotent data which would break the snapshots (e.g. date/time stamps)
You may be able to parse the JSON in to an object and assert on specific nodes of the object or fuzzy match where needed

Oooo yes I see what you mean. I don't mind what you've done with it though. 🎉

There are a couple of fields I had to miss out, because the key is dynamically generated e.g.

    "ValidateIsinLambdaVersion6F1yFivwUErVb4oeRL4yIAVELyFwfsjK7XsJuVKpg": {
      "Type": "AWS::Lambda::Version",
      "DeletionPolicy": "Retain",
      "Properties": {
        "FunctionName": {
          "Ref": "ValidateIsinLambdaFunction"
        },
        "CodeSha256": "e3rFVkcGNQQ/f5FlNP1UrgUuW453BrlyaenA0ZTb2Yk="
      }
    },

samchungy
samchungy previously approved these changes Mar 26, 2022
@mishabruml
Copy link
Contributor Author

hey can I bump @floydspace for a review 😄 🙏

Copy link
Owner

@floydspace floydspace left a comment

Choose a reason for hiding this comment

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

Sorry @mishabruml for taking it too long from my side, I approve it

@samchungy samchungy merged commit 8db0df8 into floydspace:master May 13, 2022
@samchungy
Copy link
Collaborator

Thanks for your hard work here!

@github-actions
Copy link

🎉 This PR is included in version 1.27.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

e2e integration tests
3 participants