-
-
Notifications
You must be signed in to change notification settings - Fork 11
Add high-level tests #92
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
base: main
Are you sure you want to change the base?
Conversation
16cb8b6
to
8d794c0
Compare
[InputKeys.ReleaseVersion]: string; | ||
} | ||
} | ||
interface ActionEnv extends NodeJS.ProcessEnv { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was changed because we access process.env
in some of the setup code the new tests and these ambient types were confusing TypeScript. We don't need to modify the global ProcessEnv type; we just need to get TypeScript to see process.env
in a certain light when we are accessing it in this file.
@@ -21,5 +21,5 @@ module.exports = { | |||
restoreMocks: true, | |||
testEnvironment: 'node', | |||
testRegex: ['\\.test\\.(ts|js)$'], | |||
testTimeout: 2500, | |||
testTimeout: 5000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've raised the timeout globally because many of the high level tests may take more than 2.5s to run (due to the fact that we are shelling out and because the main script itself takes about a half a second to run).
@@ -0,0 +1,128 @@ | |||
import path from 'path'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to leave comments describing what each thing does, but here is a high level:
- To set up a test, we need to create two repos. We will use one repo to run the action script, but we also need another repo that the first one can talk to so that the action script can run
git fetch --tags
. - The collection of these repos is called an environment and are kept in a directory within a temporary location. The environment is cleared and recreated before each test.
- There are two types of Environment classes, one which is specialized to initialize a polyrepo and one to initialize a monorepo. These have different characteristics, so there are two kinds of Repo classes as well.
e4bd9c2
to
6bf6cbc
Compare
a196974
to
6b1556b
Compare
This commit adds tests which exercise the behavior of the script that powers the majority of this action. It does this by using real Git repos and testing the contents of real files instead of mocking out commands and access to the filesystem. The tests in this commit should cover all of the happy paths provided directly by this action, as well as some behavior offered by `@metamask/auto-changelog` is also covered, because it seemed to make sense when thinking about how this action "should" work from a high level.
6b1556b
to
3c9f2e6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs conflict-resolution of yarn.lock
; LGTM otherwise from a quick look!
Done! |
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring: Next stepsTake a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with |
This commit adds tests which exercise the behavior of the script that powers the majority of this action. It does this by using real Git repos and testing the contents of real files instead of mocking out commands and access to the filesystem.
The tests in this commit should cover all of the happy paths provided directly by this action, as well as some behavior offered by
@metamask/auto-changelog
is also covered, because it seemed to make sense when thinking about how this action "should" work from a high level.The new file that this introduces,
high-level.test.ts
, has a lot of tests in it, and so it can be quite intimidating to review. That said, a lot of the tests repeat themselves. I would recommend runningyarn test
and looking at thedescribe
s andit
s to get an idea of the structure. For more help, however, here are some of the high points: