Skip to content

Commit cb12aff

Browse files
cwong-scwcwong-scw
committed
Update tests for new help object behaviour
1 parent 4eca8fd commit cb12aff

File tree

9 files changed

+445
-25
lines changed

9 files changed

+445
-25
lines changed

‎dist/index.js‎

Lines changed: 12 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/index.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎fixtures/test006.sarif‎

Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
{
2+
"$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
3+
"version": "2.1.0",
4+
"runs": [
5+
{
6+
"tool": {
7+
"driver": {
8+
"name": "Tool Name 3",
9+
"rules": [
10+
{
11+
"id": "TEST01",
12+
"name": "Test 01 rule name cwe: 23",
13+
"messageStrings": {
14+
"default": {
15+
"text": "This is the message text. It might be very long."
16+
}
17+
},
18+
"shortDescription": {
19+
"text": "short description"
20+
},
21+
"fullDescription": {
22+
"text": "full description text",
23+
"markdown": "full description markdown"
24+
},
25+
"help": {
26+
"text": "some help text",
27+
"markdown": "markdown version some link [here](https://github.com)"
28+
},
29+
"properties": {
30+
"tags": [
31+
"Tag A"
32+
]
33+
}
34+
},
35+
{
36+
"id": "TEST02",
37+
"name": "Test 02 rule name",
38+
"messageStrings": {
39+
"default": {
40+
"text": "This is the message text. It might be very long."
41+
}
42+
},
43+
"shortDescription": {
44+
"text": "short description"
45+
},
46+
"fullDescription": {
47+
"text": "full description text",
48+
"markdown": "full description markdown"
49+
},
50+
"help": {
51+
"text": "",
52+
"markdown": ""
53+
},
54+
"properties": {
55+
"tags": [
56+
"CWE 94"
57+
]
58+
}
59+
},
60+
{
61+
"id": "TEST03",
62+
"name": "Test 03 rule name",
63+
"messageStrings": {
64+
"default": {
65+
"text": "This is the message text. It might be very long."
66+
}
67+
},
68+
"shortDescription": {
69+
"text": "short description"
70+
},
71+
"fullDescription": {
72+
"text": "full description text",
73+
"markdown": "full description markdown"
74+
},
75+
"properties": {
76+
"tags": [
77+
"CWE 123"
78+
]
79+
}
80+
},
81+
{
82+
"id": "TEST04",
83+
"name": "Test 04 rule name",
84+
"messageStrings": {
85+
"default": {
86+
"text": "This is the message text. It might be very long."
87+
}
88+
},
89+
"shortDescription": {
90+
"text": "short description"
91+
},
92+
"fullDescription": {
93+
"text": "full description text"
94+
},
95+
"properties": {
96+
"tags": [
97+
"CWE 234"
98+
]
99+
}
100+
},
101+
{
102+
"id": "TEST05",
103+
"name": "Test 05 rule name",
104+
"messageStrings": {
105+
"default": {
106+
"text": "This is the message text. It might be very long."
107+
}
108+
},
109+
"shortDescription": {
110+
"text": "short description"
111+
},
112+
"fullDescription": {
113+
"markdown": "full description markdown"
114+
},
115+
"properties": {
116+
"tags": [
117+
"CWE 345"
118+
]
119+
}
120+
}
121+
]
122+
}
123+
},
124+
"results": [
125+
{
126+
"ruleId": "TEST01",
127+
"level": "error",
128+
"message": {
129+
"text": "Result text. This result does not have a rule associated."
130+
},
131+
"locations": [
132+
{
133+
"physicalLocation": {
134+
"artifactLocation": {
135+
"uri": "app.js"
136+
},
137+
"region": {
138+
"startLine": 5,
139+
"startColumn": 4,
140+
"endColumn": 10
141+
}
142+
}
143+
}
144+
],
145+
"partialFingerprints": {
146+
"primaryLocationLineHash": "39fa2ee980eb94b0:1"
147+
}
148+
},
149+
{
150+
"ruleId": "TEST02",
151+
"level": "note",
152+
"message": {
153+
"text": "more different text."
154+
},
155+
"locations": [
156+
{
157+
"physicalLocation": {
158+
"artifactLocation": {
159+
"uri": "app.js"
160+
},
161+
"region": {
162+
"startLine": 15,
163+
"startColumn": 5,
164+
"endColumn": 8
165+
}
166+
}
167+
}
168+
],
169+
"partialFingerprints": {
170+
"primaryLocationLineHash": "39fa2ee980eb94d0:1"
171+
}
172+
},
173+
{
174+
"ruleId": "TEST03",
175+
"level": "note",
176+
"message": {
177+
"text": "more different text."
178+
},
179+
"locations": [
180+
{
181+
"physicalLocation": {
182+
"artifactLocation": {
183+
"uri": "app.js"
184+
},
185+
"region": {
186+
"startLine": 115,
187+
"startColumn": 15,
188+
"endColumn": 18
189+
}
190+
}
191+
}
192+
],
193+
"partialFingerprints": {
194+
"primaryLocationLineHash": "39fa2ee980eb94d0:2"
195+
}
196+
},
197+
{
198+
"ruleId": "TEST04",
199+
"level": "note",
200+
"message": {
201+
"text": "more different text."
202+
},
203+
"locations": [
204+
{
205+
"physicalLocation": {
206+
"artifactLocation": {
207+
"uri": "app.js"
208+
},
209+
"region": {
210+
"startLine": 215,
211+
"startColumn": 25,
212+
"endColumn": 28
213+
}
214+
}
215+
}
216+
],
217+
"partialFingerprints": {
218+
"primaryLocationLineHash": "39fa2ee980eb94d0:3"
219+
}
220+
},
221+
{
222+
"ruleId": "TEST05",
223+
"level": "note",
224+
"message": {
225+
"text": "more different text."
226+
},
227+
"locations": [
228+
{
229+
"physicalLocation": {
230+
"artifactLocation": {
231+
"uri": "app.js"
232+
},
233+
"region": {
234+
"startLine": 315,
235+
"startColumn": 35,
236+
"endColumn": 38
237+
}
238+
}
239+
}
240+
],
241+
"partialFingerprints": {
242+
"primaryLocationLineHash": "39fa2ee980eb94d0:4"
243+
}
244+
}
245+
]
246+
}
247+
]
248+
}

‎sarifLoader.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,15 @@ test('sarifLoader should better load test001 and retrieve version', async () =>
1212

1313
test('sarifLoader should better load fixtures directory and retrieve versions from each file', async () => {
1414
const sarifs = await sarifLoader.load('./fixtures');
15-
expect(sarifs.length).toEqual(6);
15+
expect(sarifs.length).toEqual(7);
1616
for (const sarif of sarifs) {
1717
expect(sarif.version).toEqual('2.1.0');
1818
}
1919
});
2020

2121
test('sarifLoader should better load fixtures directory with trailing slash and retrieve versions from each file', async () => {
2222
const sarifs = await sarifLoader.load('./fixtures/');
23-
expect(sarifs.length).toEqual(6);
23+
expect(sarifs.length).toEqual(7);
2424
for (const sarif of sarifs) {
2525
expect(sarif.version).toEqual('2.1.0');
2626
}

‎sarifProcessors/helpProcessor.js‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
"use strict";
22

33
function addTextAndMarkdown(helpObj, textToAdd, markdownToAdd) {
4-
if (helpObj && !helpObj.text && !helpObj.markdown) {
5-
helpObj.text = textToAdd;
6-
helpObj.markdown = markdownToAdd;
7-
return;
8-
}
9-
4+
// This will blindly add to both text and markdown but the help object supplied will always have both
105
if (helpObj && helpObj.text) {
116
helpObj.text += `\n\n${textToAdd}`;
127
}
8+
else if (helpObj && !helpObj.text) {
9+
helpObj.text = textToAdd;
10+
}
1311

1412
if (helpObj && helpObj.markdown) {
1513
helpObj.markdown += `\n\n${markdownToAdd}`;
1614
}
15+
else if (helpObj && !helpObj.markdown) {
16+
helpObj.markdown = markdownToAdd;
17+
}
1718
}
1819

1920
function appendHeader(helpObj) {

‎sarifProcessors/helpProcessor.test.js‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,32 +18,35 @@ test('helpProcessor should create both text and markdown if neither are present'
1818
});
1919
});
2020

21-
test('helpProcessor should append to markdown if only markdown is present', async () => {
21+
test('helpProcessor should append to markdown and add text if only markdown is present', async () => {
2222
const helpObj = {
2323
markdown: 'existing markdown'
2424
};
2525
helpProcessor.appendTrainingData(helpObj, NAME, DESCRIPTION, URL, VIDEOS, DISPLAY_REF);
2626
expect(helpObj).toEqual({
27-
markdown: `existing markdown\n\n#### [${DISPLAY_REF}] ${NAME} *[What is this? (2min video)](${VIDEOS[0]})*\n\n* ${DESCRIPTION} [Try this challenge in Secure Code Warrior](${URL})`
27+
markdown: `existing markdown\n\n#### [${DISPLAY_REF}] ${NAME} *[What is this? (2min video)](${VIDEOS[0]})*\n\n* ${DESCRIPTION} [Try this challenge in Secure Code Warrior](${URL})`,
28+
text: `[${DISPLAY_REF}] ${NAME} [What is this? (2min video)](${VIDEOS[0]})\n\n${DESCRIPTION} [Try this challenge in Secure Code Warrior](${URL})`
2829
});
2930
});
3031

31-
test('helpProcessor should append to markdown if only markdown is present (no video case)', async () => {
32+
test('helpProcessor should append to markdown and add text if only markdown is present (no video case)', async () => {
3233
const helpObj = {
3334
markdown: 'existing markdown'
3435
};
3536
helpProcessor.appendTrainingData(helpObj, NAME, DESCRIPTION, URL, NO_VIDEO, DISPLAY_REF);
3637
expect(helpObj).toEqual({
37-
markdown: `existing markdown\n\n#### [${DISPLAY_REF}] ${NAME}\n\n* ${DESCRIPTION} [Try this challenge in Secure Code Warrior](${URL})`
38+
markdown: `existing markdown\n\n#### [${DISPLAY_REF}] ${NAME}\n\n* ${DESCRIPTION} [Try this challenge in Secure Code Warrior](${URL})`,
39+
text: `[${DISPLAY_REF}] ${NAME}\n\n${DESCRIPTION} [Try this challenge in Secure Code Warrior](${URL})`
3840
});
3941
});
4042

41-
test('helpProcessor should append to text if only text is present', async () => {
43+
test('helpProcessor should append to text and add markdown if only text is present', async () => {
4244
const helpObj = {
4345
text: 'existing text'
4446
};
4547
helpProcessor.appendTrainingData(helpObj, NAME, DESCRIPTION, URL, VIDEOS, DISPLAY_REF);
4648
expect(helpObj).toEqual({
49+
markdown: `#### [${DISPLAY_REF}] ${NAME} *[What is this? (2min video)](${VIDEOS[0]})*\n\n* ${DESCRIPTION} [Try this challenge in Secure Code Warrior](${URL})`,
4750
text: `existing text\n\n[${DISPLAY_REF}] ${NAME} [What is this? (2min video)](${VIDEOS[0]})\n\n${DESCRIPTION} [Try this challenge in Secure Code Warrior](${URL})`
4851
});
4952
});

‎sarifProcessors/ruleProcessor.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,12 @@ async function process(run, languageKey, triggeredRules) {
5757
}
5858

5959
if (!rule.help) rule.help = {
60+
// if `help` is not present but fullDescription is present
61+
// init `help` with `fullDescription` to avoid overwriting the displayed description
62+
// for `markdown` fallback to `text` if there is no `fullDescription.markdown`
63+
// for `text` fallback to "No description" if there is no `fullDescription.text`
6064
text: (rule.fullDescription && rule.fullDescription.text) || '',
61-
markdown: (rule.fullDescription && rule.fullDescription.markdown) || ''
65+
markdown: (rule.fullDescription && (rule.fullDescription.markdown || rule.fullDescription.text)) || ''
6266
};
6367

6468
if (!isShown) {

0 commit comments

Comments
 (0)