Skip to content

Commit 15fc72f

Browse files
Merge pull request #9 from opengrep/fix-wrong-test-output-and-style-issues
fix: wrong tests information + fixed styling issues
2 parents 09dd5af + 510ca4a commit 15fc72f

File tree

3 files changed

+108
-66
lines changed

3 files changed

+108
-66
lines changed

src/components/CodeEditor.vue

Lines changed: 97 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const MONACO_EDITOR_OPTIONS = {
2727
glyphMargin: true,
2828
lineNumbersMinChars: 3,
2929
scrollbar: {
30-
vertical: 'hidden',
30+
vertical: 'auto',
3131
horizontal: 'hidden'
3232
}
3333
};
@@ -66,6 +66,11 @@ watch(() => store.codeEditorCode, (newCode) => {
6666
if (model && model.getValue() !== newCode) {
6767
model.setValue(newCode);
6868
}
69+
70+
editorRef.value.changeViewZones(accessor => {
71+
// Remove existing annotation zones
72+
componentState.exisitingAnnotationZones.forEach(zone => accessor.removeZone(zone));
73+
});
6974
}
7075
}, { deep: true });
7176
@@ -104,95 +109,125 @@ function determineHighlightLinesFromTestResult(rawTestResults) {
104109
if (!rawTestResults || !Object.entries(rawTestResults.results).length) return;
105110
106111
let newDecorations = [];
107-
store.jsonResult.parsedTestResults = [];
108-
109-
// Check for comments indicating false positives (//ok or // ok)
110-
const model = editorRef.value.getModel();
111-
const linesCount = model.getLineCount();
112-
for (let lineNumber = 1; lineNumber <= linesCount; lineNumber++) {
113-
const lineContent = model.getLineContent(lineNumber);
114-
if (lineContent.includes('ok: ') || lineContent.includes('ruleid: ')) {
115-
store.jsonResult.parsedTestResults.push({
116-
mustMatch: false,
117-
lineNumber: ++lineNumber,
118-
ruleId: null,
119-
status: 'SUCCESS'
120-
});
121-
122-
newDecorations.push({
123-
range: new monaco.Range(lineNumber - 1, 1, lineNumber - 1, 1),
124-
options: {
125-
isWholeLine: true,
126-
className: "full-line-highlight-added",
127-
glyphMarginClassName: "diff-added-gutter"
128-
}
129-
});
130-
}
131-
}
132-
112+
store.jsonResult.parsedTestResults = [];
113+
let expectedSet = null;
114+
let reportedSet = null;
133115
134116
// Extract the matches from the JSON
135117
Object.entries(rawTestResults.results[store.ruleFilePath].checks).forEach(([ruleId, check]) => {
136118
Object.entries(check.matches || {}).forEach(([, matchData]) => {
137119
const { expected_lines = [], reported_lines = [] } = matchData;
138120
139121
// Convert arrays to sets for easier comparison
140-
const expectedSet = new Set(expected_lines);
141-
const reportedSet = new Set(reported_lines);
142-
143-
// Highlight expected lines (Green - should be present)
122+
expectedSet = new Set(expected_lines);
123+
reportedSet = new Set(reported_lines);
124+
125+
/** if expected but not reported then
126+
* ok ==> must not match green
127+
* ruleid ==> must match red
128+
* nothing ==> empty
129+
*/
144130
expected_lines.forEach((line) => {
145-
newDecorations = newDecorations.map(decoration => {
146-
if (decoration.range.startLineNumber === line - 1) {
147-
return {
148-
...decoration,
149-
options: {
150-
...decoration.options,
151-
className: reportedSet.has(line) ? "full-line-highlight-added" : "full-line-highlight-removed",
152-
glyphMarginClassName: reportedSet.has(line) ? "diff-added-gutter" : "diff-removed-gutter"
153-
}
131+
if (!reportedSet.has(line)) {
132+
newDecorations.push({
133+
range: new monaco.Range(line - 1, 1, line - 1, 1),
134+
options: {
135+
isWholeLine: true,
136+
className: "full-line-highlight-removed",
137+
glyphMarginClassName: "diff-removed-gutter"
154138
}
155-
}
156-
return decoration;
157-
});
158-
159-
store.jsonResult.parsedTestResults = store.jsonResult.parsedTestResults.map(testResult => {
160-
if (testResult.lineNumber === line) {
161-
return {
162-
...testResult,
163-
mustMatch: reportedSet.has(line),
164-
ruleId,
165-
status: reportedSet.has(line) ? 'SUCCESS' : 'FAILED'
166-
};
167-
}
139+
});
168140
169-
return testResult;
170-
});
141+
store.jsonResult.parsedTestResults.push({
142+
lineNumber: line,
143+
mustMatch: true,
144+
ruleId,
145+
status: 'FAILED'
146+
});
147+
}
171148
});
172149
173-
// Highlight reported lines that are not in expected lines (Red - detected issues)
174-
reported_lines.forEach(line => {
150+
/** if reported but not expected then
151+
* ok comment ==> must not match red
152+
* ruleid comment ==> must match green
153+
* no comment ==> empty
154+
*/
155+
reported_lines.forEach((line) => {
175156
if (!expectedSet.has(line)) {
176157
newDecorations.push({
177-
range: new monaco.Range(line, 1, line, 1),
158+
range: new monaco.Range(line - 1, 1, line - 1, 1),
178159
options: {
179160
isWholeLine: true,
180-
className: "full-line-highlight-unexpected",
181-
glyphMarginClassName: "diff-unexpected-gutter"
161+
className: "full-line-highlight-removed",
162+
glyphMarginClassName: "diff-removed-gutter"
182163
}
183164
});
184165
185166
store.jsonResult.parsedTestResults.push({
186167
lineNumber: line,
187168
mustMatch: false,
188169
ruleId,
189-
status: 'UNTESTED'
190-
})
170+
status: 'FAILED'
171+
});
172+
}
173+
});
174+
175+
/** if reported and expected then
176+
* ok ==> must not match red
177+
* ruleid ==> must match green
178+
* nothing ==> empty
179+
*/
180+
expected_lines.forEach((line) => {
181+
if (reportedSet.has(line)) {
182+
newDecorations.push({
183+
range: new monaco.Range(line - 1, 1, line - 1, 1),
184+
options: {
185+
isWholeLine: true,
186+
className: "full-line-highlight-added",
187+
glyphMarginClassName: "diff-added-gutter"
188+
}
189+
});
190+
191+
store.jsonResult.parsedTestResults.push({
192+
lineNumber: line,
193+
mustMatch: true,
194+
ruleId,
195+
status: 'SUCCESS',
196+
});
191197
}
192198
});
193199
});
200+
201+
/**
202+
* if no reported an no expected but indicated with the ignore command (ok: ) then
203+
* highlight as green not match
204+
*/
205+
const model = editorRef.value.getModel();
206+
const linesCount = model.getLineCount();
207+
for (let lineNumber = 1; lineNumber <= linesCount; lineNumber++) {
208+
const previousLineContent = model.getLineContent(lineNumber);
209+
if (previousLineContent.includes('ok: ') && !expectedSet.has(lineNumber + 1) && !reportedSet.has(lineNumber + 1)) {
210+
store.jsonResult.parsedTestResults.push({
211+
mustMatch: false,
212+
lineNumber: lineNumber + 1,
213+
ruleId: null,
214+
status: 'SUCCESS'
215+
});
216+
217+
newDecorations.push({
218+
range: new monaco.Range(lineNumber, 1, lineNumber, 1),
219+
options: {
220+
isWholeLine: true,
221+
className: "full-line-highlight-added",
222+
glyphMarginClassName: "diff-added-gutter"
223+
}
224+
});
225+
}
226+
}
194227
});
195228
229+
store.jsonResult.parsedTestResults.sort((a, b) => a.lineNumber - b.lineNumber);
230+
196231
// Apply decorations in Monaco Editor
197232
componentState.existingHighlightLinesFromTestResult = editorRef.value.deltaDecorations(componentState.existingHighlightLinesFromTestResult, newDecorations);
198233
}

src/components/RuleEditor.vue

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ const MONACO_EDITOR_OPTIONS = {
2727
renderWhitespace: "boundary",
2828
glyphMargin: true,
2929
lineNumbersMinChars: 3,
30+
scrollbar: {
31+
vertical: 'auto',
32+
horizontal: 'hidden'
33+
}
3034
};
3135
const languageMappings = {
3236
js: { ext: "js", monaco: "javascript" },

src/components/RuleResults.vue

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@
4040
</div>
4141

4242
<!-- TEST RESULTS -->
43-
<h4 @click="toggleSection('testResults')" style="cursor: pointer;">Test Results </h4>
43+
<h4 @click="toggleSection('testResults')" style="cursor: pointer;">
44+
Test Results ({{ store.jsonResult?.parsedTestResults.filter(test => test.status === 'SUCCESS').length }}/{{ store.jsonResult?.parsedTestResults.length }} Unit Tests Passed)
45+
</h4>
4446
<div style="flex: 1" class="scrollable-section">
4547
<div v-if="isTestLoading" class="loading-container">
4648
<div class="loading-circle"></div>
@@ -116,8 +118,8 @@ async function handleRunBinary() {
116118
try {
117119
await runBinaryForTests(binaryPath);
118120
} catch (error) {
119-
showErrorDialog(`Error running tests: Please consult the error.log file at ${store.safeDir}`, error);
120121
console.error("Error running tests:", error);
122+
showErrorDialog(`Error running tests: Please consult the error.log file at ${store.safeDir}`, error);
121123
} finally {
122124
isTestLoading.value = false;
123125
}
@@ -127,9 +129,9 @@ async function handleRunBinary() {
127129
try {
128130
await runBinaryForScan(binaryPath, false);
129131
} catch (error) {
132+
console.error("Error running scanning:", error);
130133
if (!retryAttempted) {
131134
showErrorDialog(`Error running scanning: Please consult the error.log file at ${store.safeDir}`, error);
132-
console.error("Error running scanning:", error);
133135
return;
134136
}
135137
@@ -152,6 +154,7 @@ async function runBinaryForScan(binaryPath, runScanWithoutMatchingExplanations)
152154
if (platform.value === 'win32') {
153155
scanArgs.push('-j 1');
154156
}
157+
155158
if (!runScanWithoutMatchingExplanations) {
156159
scanArgs.push('--matching-explanations');
157160
}
@@ -180,7 +183,7 @@ function extractScanErrors(jsonOutput) {
180183
}
181184
182185
async function runBinaryForTests(binaryPath) {
183-
const testArgs = ['test', `-f "${store.ruleFilePath}" "${store.codeSampleFilePath}"`, '--json', '--experimental'];
186+
const testArgs = ['scan --test', `-f "${store.ruleFilePath}" "${store.codeSampleFilePath}"`, '--json', '--experimental'];
184187
185188
const testResponse = await runBinary(`"${binaryPath}"`, testArgs)
186189
const testResults = JSON.parse(testResponse.output);

0 commit comments

Comments
 (0)