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

Add integration tests for main.js #56

Merged
merged 21 commits into from
Oct 6, 2023
Merged

Add integration tests for main.js #56

merged 21 commits into from
Oct 6, 2023

Conversation

smockle
Copy link
Contributor

@smockle smockle commented Oct 3, 2023

Part of #43

This PR adds tests for main.js, similar to the tests that already exist for post.js.

Specifically, it tests that:

  • main exits with an error when GITHUB_REPOSITORY is missing.
  • main exits with an error when GITHUB_REPOSITORY_OWNER is missing.
  • main successfully obtains a token when…
    • …the owner and repositories inputs are set (and the latter is a single repo).
    • …the owner and repositories inputs are set (and the latter is a list of repos).
    • …the owner input is set (to an org), but the repositories input isn’t set.
    • …the owner input is set (to a user), but the repositories input isn’t set.
    • …the owner input is not set, but the repositories input is set.
    • …neither the owner nor repositories input is set.

Architecturally, in order to keep individual tests concise, this PR adds tests/main.js, which:

  • sets commonly-used inputs, environment variables, and mocks, then
  • calls a callback function that can edit the variables and add additional mocks, then
  • runs main.js itself.

The tests/main-token-get-*.test.js test files run tests/main.js with various scenario-specific callback functions.

Copy link
Contributor

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

I prefer we don't use any module mocking, instead mock all requests and capture and snapshot all logs / thrown errors. Let me know if you need help with that

@smockle
Copy link
Contributor Author

smockle commented Oct 4, 2023

I prefer we don't use any module mocking…

👍 Cool; removed esmock and tests using module mocks, in https://github.com/smockle/create-github-app-token/pull/56/commits/60504269230a68706c8bf9ed9e40ced513146235.

…instead mock all requests and capture and snapshot all logs / thrown errors.…

I added a happy path test (that mocks requests and validates stdout), in https://github.com/smockle/create-github-app-token/pull/56/commits/43f24998742aeb378c1e91e0fe6a0d790a0a6b92. @gr2m, is that what you had in mind?

@gr2m
Copy link
Contributor

gr2m commented Oct 4, 2023

I added a happy path test (that mocks requests and validates stdout), in 43f2499. @gr2m, is that what you had in mind?

Yes, I just looked at it, looks good 👍🏼 Thank you!

@smockle

This comment was marked as outdated.

@smockle smockle marked this pull request as ready for review October 6, 2023 14:28
@smockle smockle requested review from parkerbxyz and a team as code owners October 6, 2023 14:28
@smockle smockle requested a review from gr2m October 6, 2023 14:28
@smockle smockle marked this pull request as draft October 6, 2023 14:47
@smockle smockle marked this pull request as ready for review October 6, 2023 14:59
@smockle smockle changed the title [WIP] Add tests for 'main.js' Add tests for 'main.js' Oct 6, 2023
@smockle smockle changed the title Add tests for 'main.js' Add integration tests for 'main.js' Oct 6, 2023
@smockle smockle changed the title Add integration tests for 'main.js' Add integration tests for main.js Oct 6, 2023
@smockle

This comment was marked as outdated.

@smockle

This comment was marked as outdated.

@smockle

This comment was marked as resolved.

Copy link
Contributor

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

just a nit that we an also address later 👍🏼

tests/main-missing-owner.test.js Outdated Show resolved Hide resolved
Copy link
Contributor

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

great, let's ship it 👍🏼

@gr2m gr2m merged commit 9b28355 into actions:main Oct 6, 2023
2 checks passed
@smockle smockle deleted the main-tests branch October 6, 2023 20:12
@create-app-token-action-releaser

🎉 This PR is included in version 1.5.0 🎉

The release is available on GitHub release

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.

2 participants