Skip to content

Commit 2c25bd6

Browse files
tjprescotttjprescott
committed
Fix up tests.
1 parent ed3dd48 commit 2c25bd6

File tree

4 files changed

+108
-71
lines changed

4 files changed

+108
-71
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## 0.2.0 (TBD)
44

5+
- Fixed issue where relative references would sometimes be resolved incorrectly. `--lhs-root`
6+
and `--rhs-root` are still needed when compiling TypeSpec. See README.md.
57
- Arrays of strings are now sorted to ensure that differences in sorting do not trigger diffs.
68
- Fixed issue where local references with `-` character would not be resolved.
79
- Using `--group-violations` will auto-generate groupings.

‎README.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ against the one generated from TypeSpec to determine if the TypeSpec accurately
2828
containing them. If the paths are folders, the tool will search for all Swagger files in that folder,
2929
but will not search subfolders.
3030

31+
`lhs_root` and `rhs_root` are optional parameters. If you are pointing to Swagger files, they should not be needed. However, if you are compiling
32+
TypeSpec, you should provide this so that the references get generated and resolve correctly. The value should be to set to where the Swagger
33+
files should be generated. Since that would normally result in overwriting the existing files, it's recommended that you just replace the path
34+
segment "stable" or "preview" with "generated" in the path. In this way, the Swagger will be generated into a unique folder with the same relative
35+
references as if it had been generated in the "correct" folder. This allows the tool to resolve the references correctly.
36+
3137
### Options
3238

3339
- `--compile-tsp`: The tool will attempt to compile TypeSpec files to Swagger using the
@@ -51,6 +57,10 @@ but will not search subfolders.
5157
these, for example to compare the definitions themselves between two Swaggers, provide this flag.
5258
- `--flatten-paths`: The default format of paths in the output is an array of path segments. If you prefer
5359
to flatten these into a forward-slash delimited string, provide this flag.
60+
- `--lhs-root`: The root path for the left-hand side Swagger files. This is used to resolve references
61+
when compiling TypeSpec files. If omitted, the tool will attempt to resolve the references without it.
62+
- `--rhs-root`: The root path for the right-hand side Swagger files. This is used to resolve references
63+
when compiling TypeSpec files. If omitted, the tool will attempt to resolve the references without it.
5464

5565
### .env File
5666

@@ -111,7 +121,9 @@ These steps assumed that `--lhs` points to a Swagger folder and `--rhs` points t
111121
1. Ensure you have updated the dependencies in your fork by running `npm install` in the REST API specs repo root. You may need to delete `package-lock.json` first. Copy the path to the `node_modules/@typespec/compiler` package.
112122
2. Set the `TYPESPEC_COMPILER_PATH` environment variable (ideally in .env) to the path you copied in step 1.
113123
3. Ensure that LHS and RHS point to the appropriate paths in the REST API specs repo.
114-
4. If you are comparing to a multi-versioned TypeSpec, you should probably include the `TYPESPEC_VERSION_SELECTOR` environment variable to ensure you are generating the right version for comparison.
124+
4. By convention, if you are comparing hand-written Swagger to TypeSpec, the Swagger should be LHS and the TypeSpec should be RHS. When compiling TypeSpec, you will
125+
need to set RHS_ROOT. It will generally be the same path as LHS, but with the "stable" or "preview" segment replaced with "generated".
126+
5. If you are comparing to a multi-versioned TypeSpec, you should probably include the `TYPESPEC_VERSION_SELECTOR` environment variable to ensure you are generating the right version for comparison.
115127

116128
## Rules
117129

‎test/files/typespecMulti/openapi.json

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
{
2+
"swagger": "2.0",
3+
"info": {
4+
"title": "(title)",
5+
"version": "0000-00-00",
6+
"x-typespec-generated": [
7+
{
8+
"emitter": "@azure-tools/typespec-autorest"
9+
}
10+
]
11+
},
12+
"schemes": [
13+
"https"
14+
],
15+
"produces": [
16+
"application/json"
17+
],
18+
"consumes": [
19+
"application/json"
20+
],
21+
"tags": [],
22+
"paths": {
23+
"/": {
24+
"get": {
25+
"operationId": "Get",
26+
"parameters": [],
27+
"responses": {
28+
"200": {
29+
"description": "The request has succeeded.",
30+
"schema": {
31+
"$ref": "#/definitions/Foo"
32+
}
33+
}
34+
}
35+
},
36+
"post": {
37+
"operationId": "Create",
38+
"parameters": [
39+
{
40+
"name": "body",
41+
"in": "body",
42+
"required": true,
43+
"schema": {
44+
"type": "object",
45+
"properties": {
46+
"body": {
47+
"$ref": "#/definitions/Foo"
48+
}
49+
},
50+
"required": [
51+
"body"
52+
]
53+
}
54+
}
55+
],
56+
"responses": {
57+
"200": {
58+
"description": "The request has succeeded.",
59+
"schema": {
60+
"$ref": "#/definitions/Foo"
61+
}
62+
}
63+
}
64+
}
65+
}
66+
},
67+
"definitions": {
68+
"Foo": {
69+
"type": "object",
70+
"properties": {
71+
"name": {
72+
"type": "string"
73+
},
74+
"age": {
75+
"type": "integer",
76+
"format": "int16"
77+
},
78+
"weight": {
79+
"type": "number",
80+
"format": "float"
81+
}
82+
},
83+
"required": [
84+
"name",
85+
"age",
86+
"weight"
87+
]
88+
}
89+
},
90+
"parameters": {}
91+
}

‎test/rest-api-diff.test.ts

Lines changed: 2 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ it("config should group violations when --group-violations is set", async () =>
3030
"Changed_format (AUTO)",
3131
"Added_favoriteColor (AUTO)",
3232
"ArrayItem_Added_required (AUTO)",
33+
"Changed_required (AUTO)",
3334
]);
3435

3536
const diffInvPath = path.join(tempDir, "diff-inv.json");
@@ -44,7 +45,7 @@ it("config should group violations when --group-violations is set", async () =>
4445
const invCounts = Object.values(diffInvFile).map(
4546
(item: any) => item.items.length
4647
);
47-
expect(invCounts).toStrictEqual([7, 3]);
48+
expect(invCounts).toStrictEqual([7, 4]);
4849
expect(invCounts).toStrictEqual(invCounts.sort());
4950

5051
// ensure name is removed from each value
@@ -293,72 +294,3 @@ it("should sort arrays of strings for stable comparison", async () => {
293294
expect(client.diffResults?.noViolations.length).toBe(0);
294295
expect(client.diffResults?.assumedViolations.length).toBe(0);
295296
});
296-
297-
it("should propagate suppressions that are expanded during parsing", async () => {
298-
const config: DiffClientConfig = {
299-
lhs: ["test/files/suppressions1a.json"],
300-
rhs: ["test/files/suppressions1b.json"],
301-
args: {
302-
suppressions: "test/files/suppressions1.yaml",
303-
},
304-
rules: getApplicableRules({}),
305-
};
306-
const client = await TestableDiffClient.create(config);
307-
client.parse();
308-
client.processDiff();
309-
client.buildOutput();
310-
const [lhsParser, rhsParser] = client.getParsers();
311-
expect(lhsParser.getUnresolvedReferences().length).toBe(0);
312-
expect(lhsParser.getUnreferencedTotal()).toBe(0);
313-
expect(rhsParser.getUnresolvedReferences().length).toBe(0);
314-
expect(rhsParser.getUnreferencedTotal()).toBe(0);
315-
expect(client.diffResults?.assumedViolations.length).toBe(0);
316-
expect(client.diffResults?.flaggedViolations.length).toBe(0);
317-
expect(client.diffResults?.noViolations.length).toBe(1);
318-
});
319-
320-
it("should propagate suppressions that are expanded while collecting definitions", async () => {
321-
const config: DiffClientConfig = {
322-
lhs: ["test/files/suppressions2a.json"],
323-
rhs: ["test/files/suppressions2b.json"],
324-
args: {
325-
suppressions: "test/files/suppressions2.yaml",
326-
},
327-
rules: getApplicableRules({}),
328-
};
329-
const client = await TestableDiffClient.create(config);
330-
client.parse();
331-
client.processDiff();
332-
client.buildOutput();
333-
const [lhsParser, rhsParser] = client.getParsers();
334-
expect(lhsParser.getUnresolvedReferences().length).toBe(0);
335-
expect(lhsParser.getUnreferencedTotal()).toBe(0);
336-
expect(rhsParser.getUnresolvedReferences().length).toBe(0);
337-
expect(rhsParser.getUnreferencedTotal()).toBe(0);
338-
expect(client.diffResults?.assumedViolations.length).toBe(0);
339-
expect(client.diffResults?.flaggedViolations.length).toBe(0);
340-
expect(client.diffResults?.noViolations.length).toBe(2);
341-
});
342-
343-
it("should propagate suppressions for circular references", async () => {
344-
const config: DiffClientConfig = {
345-
lhs: ["test/files/suppressions3a.json"],
346-
rhs: ["test/files/suppressions3b.json"],
347-
args: {
348-
suppressions: "test/files/suppressions3.yaml",
349-
},
350-
rules: getApplicableRules({}),
351-
};
352-
const client = await TestableDiffClient.create(config);
353-
client.parse();
354-
client.processDiff();
355-
client.buildOutput();
356-
const [lhsParser, rhsParser] = client.getParsers();
357-
expect(lhsParser.getUnresolvedReferences().length).toBe(0);
358-
expect(lhsParser.getUnreferencedTotal()).toBe(0);
359-
expect(rhsParser.getUnresolvedReferences().length).toBe(0);
360-
expect(rhsParser.getUnreferencedTotal()).toBe(0);
361-
expect(client.diffResults?.assumedViolations.length).toBe(0);
362-
expect(client.diffResults?.flaggedViolations.length).toBe(0);
363-
expect(client.diffResults?.noViolations.length).toBe(1);
364-
});

0 commit comments

Comments
 (0)