Skip to content

Conversation

mcmire
Copy link
Contributor

@mcmire mcmire commented Jul 20, 2022

This commit adds tests which run the script within a separate shell
instance and verifies that the script makes the appropriate changes to
the repo.

I've tried to make these functional tests as readable as possible. In
order to do that, I created a small framework which replicates the
environment in which the script will run and then allows that
environment to be modified per-test to suit the assertions being made.

@mcmire mcmire requested a review from a team as a code owner July 20, 2022 21:50
@mcmire mcmire self-assigned this Jul 25, 2022
Base automatically changed from add-first-flow to main August 2, 2022 22:24
@mcmire mcmire force-pushed the add-functional-tests branch from 30abbd6 to 120a622 Compare August 2, 2022 22:44
This commit adds tests which run the script within a separate shell
instance and verifies that the script makes the appropriate changes to
the repo.

I've tried to make these functional tests as readable as possible. In
order to do that, I created a small framework which replicates the
environment in which the script will run and then allows that
environment to be modified per-test to suit the assertions being made.
@mcmire mcmire force-pushed the add-functional-tests branch from 120a622 to e23c2e2 Compare August 2, 2022 23:06
* @param error - The object to check.
* @returns True or false, depending on the result.
*/
export function isErrorWithCode(error: unknown): error is { code: string } {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You'll notice that some of these helpers are repeated from misc-utils. I could have reused them, but it feels a little off to me to reuse implementation code in test code, even if it's the exact same code. Ideally, I think, we could move this to @metamask/action-utils or even @metamask/utils and then I think I'd feel better about it.


describe('create-release-branch (functional)', () => {
describe('against a monorepo with independent versions', () => {
it('updates the version of the root package to be the current date along with the versions of the specified packages', async () => {
Copy link
Contributor Author

@mcmire mcmire Aug 2, 2022

Choose a reason for hiding this comment

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

I'm only testing the happy paths right now. This file is going to grow larger as we add different features — for instance, right now there is no logic around changed packages, so once that happens we need to consider what happens when the repo has no tags, etc. — so I'm hesitant to add error/alternate cases. I figure it's easier to add more test cases later than remove them.

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

This looks great! The test helpers here are... extensive, but the way it's structured all made sense to me. It might have been easier to use plain file fixtures instead for the workspace/repo generation parts, but I can see some advantages to this way too.

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@mcmire mcmire merged commit d3dd20a into main Aug 5, 2022
@mcmire mcmire deleted the add-functional-tests branch August 5, 2022 03:46
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.

2 participants