rsm-hcd / AndcultureCode.Cli

and-cli command-line tool to manage the development of software applications
https://andculture.com
Apache License 2.0
14 stars 15 forks source link

Support/extract tested modules #55

Closed brandongregoryscott closed 4 years ago

brandongregoryscott commented 4 years ago

Motivation

In an effort to more effectively test the business logic in the commands that we're adding or modifying in the CLI, and to better set us up for integrating with TravisCI, I have extracted the two existing test files (and their implementations) into the _modules folder. (dotnet-test and nuget-upgrade, specifically)

While these two modules are not being shared between commands yet, it is possible that they could be. Either way, extracting them out into modules that can be imported by the wrapper "cli-command" wrappers allows us to more easily test the logic that's happening without trying to work against the commander package.

Let me know if you need any additional context or want to know why I changed something I did. I have another branch I've been working on to get TravisCI up and running, which will follow.

brandongregoryscott commented 4 years ago

@brandongregoryscott gave it a run and it all checked out. Didn't test every possible combo of flags, but the high priority paths are good. Thanks!

I'm good to approve when you are done reviewing the feedback. Just let me know and I'll move it on its way.

Thanks for checking it out! I'll ping you when I've taken care of the feedback here.

brandongregoryscott commented 4 years ago

@wintondeshong This is ready for review again. Your initial comments as well as the 'command-runner' pattern we worked out should be implemented for every CLI method. There was some refactoring I had to do to call spawnSync instead of just spawn for the commands using it. Since spawn is async-ish, it returns immediately and cannot be awaited. The behavior would be that the commands with async-ish behavior (dotnet, dotnet-test, webpack, webpack-test) would exit immediately.

I tried to thoroughly test the rest of them that I could once I discovered this behavior, but there are certain CLI commands I am not sure how to setup/test for (mainly the deployment ones).

If you could take a look at the PR again, and pull it down to test out the critical paths yourself, I'd appreciate that. This turned into a pretty hefty refactor once it was all said and done.

Let me know if you have any questions or see something funky.

brandongregoryscott commented 4 years ago

@brandongregoryscott nice work! Sorry it took me so long to get around to reviewing. I gave this a whirl locally and primary use cases checked out. Not sure what you think, but I'm thinking we rev to v1 soon. The amount of features and work to this point have been significant enough imho.

Once you do the minor cleanup below, I'll approve and merge.

No worries! Oddly enough, I was thinking about pinging you the same day you took a look at it!

I'm fine with bumping it to a v1 soon. I agree, the commands have been built out quite a bit and battle tested with the team using it.

I've been meaning to take another pass on the documentation side of it, I believe there might be a few commands or flags that haven't been updated as the code has changed.

I'd also love to get my TravisCI branch in before we do, I think that'd be a good marker for a v1 release.

I'll work on these comments and request a review again after I've taken care of them, thanks!