feat: Detect circular dependencies. #95
Conversation
test/src/algo.ts
Outdated
| const circularProjectRoot = join(__dirname, "..", "data", "test-project-circular-deps") | ||
| const nonCircularProjectRoot = join(__dirname, "..", "data", "test-project-b") | ||
|
|
||
| describe("Algo", () => { |
There was a problem hiding this comment.
Algo is not a good name for this :) I dunno, maybe we should rename util.ts to util/index.ts and add another file under util/ with a more descriptive name.
There was a problem hiding this comment.
Haha. Agreed. I'll do that, and rename this file to util/detectCycles.ts.
test/src/algo.ts
Outdated
| it("should throw an exception when circular dependencies are present", async () => { | ||
| const garden = await makeTestGarden(circularProjectRoot) | ||
| await expectError( | ||
| async () => await garden.scanModules(), |
There was a problem hiding this comment.
I think it's better to put these tests under the Garden.scanModules tests, and in turn write a couple of basic unit tests for the detectCycles function.
test/helpers.ts
Outdated
| @@ -201,6 +201,15 @@ export async function expectError(fn: Function, typeOrCallback: string | ((err: | |||
| } | |||
| } | |||
|
|
|||
| export async function expectNoError(fn: Function) { | |||
There was a problem hiding this comment.
Isn't this redundant? Also, can't see it used anywhere.
There was a problem hiding this comment.
It is indeed, forgot to delete this one.
|
A handful of small comments, otherwise looks good! |
|
Looks like I accidentally avoided the commit formatter for the latter commit (since I wrote it while squashing). Should I replace the commit or just leave it? |
|
I'd let it slide, but it also has a typo in it and it tickles my OCD :) so yeah, best replace it |
|
Or, actually, you can just squash the whole thing. No reason to have a commit and then a fix for that commit in succession (I personally use |
thsig
force-pushed
the
detect-circular-dependencies
branch
from
d98fda5 to
9c83c1b
Compare
|
'Tis been squashed. |
test/src/util.ts
Outdated
| const circularProjectRoot = join(__dirname, "..", "data", "test-project-circular-deps") | ||
| const garden = await makeTestGarden(circularProjectRoot) | ||
| await expectError( | ||
| async () => await garden.scanModules(), |
There was a problem hiding this comment.
This test seems to be in the wrong place - it's not directly testing this function, nor does that function have any knowledge of Module specs.
There was a problem hiding this comment.
Good point - these two probably belong under describe("util") -> describe("detectCircularDependencies") in this file. Or did you have another placement in mind?
There was a problem hiding this comment.
Since this actually tests the scanModules method on Garden, it should be with those tests in tests/src/garden.ts (along with the other scanModules tests)
thsig
force-pushed
the
detect-circular-dependencies
branch
3 times, most recently
from
22c5a66 to
0eadd2c
Compare
Added a check for circular build or service dependencies to the scanModules method of the Garden class.
thsig
force-pushed
the
detect-circular-dependencies
branch
from
0eadd2c to
4a35276
Compare
Added a check for circular build or service dependencies to the
scanModules method of the Garden class.