diff --git a/tests/ui/logviewer/logviewerHelpers_test.js b/tests/ui/logviewer/logviewerHelpers_test.js new file mode 100644 index 00000000000..14bb89673f3 --- /dev/null +++ b/tests/ui/logviewer/logviewerHelpers_test.js @@ -0,0 +1,37 @@ +import { splitLogIntoLines } from '../../../ui/logviewer/logviewerHelpers'; + +describe('splitLogIntoLines', () => { + test('splits on LF (\\n)', () => { + expect(splitLogIntoLines('a\nb\nc')).toEqual(['a', 'b', 'c']); + }); + + test('splits on bare CR (\\r) the same way the backend parser does', () => { + // The backend records error line_numbers via Python bytes.splitlines(), + // which breaks on bare carriage returns. A plain split('\n') would NOT, + // leaving "a\rb" as a single line and shifting every later line number. + expect(splitLogIntoLines('a\rb\rc')).toEqual(['a', 'b', 'c']); + }); + + test('treats CRLF (\\r\\n) as a single break with no empty line or trailing CR', () => { + expect(splitLogIntoLines('a\r\nb\r\nc')).toEqual(['a', 'b', 'c']); + }); + + test('keeps the same line count across mixed CR / CRLF / LF endings', () => { + expect(splitLogIntoLines('a\nb\rc\r\nd')).toEqual(['a', 'b', 'c', 'd']); + }); + + test('a bare CR before an error line shifts subsequent line indexes (regression)', () => { + // Reproduces the highlight-offset bug: the failure line sits after a bare CR. + // With split('\n') the failure would land at index 1; the backend counts the + // CR as a line break, so the matching display index must be 2. + const log = 'header\rprogress\nTEST-UNEXPECTED-FAIL\ntrailer'; + const lines = splitLogIntoLines(log); + expect(lines).toEqual([ + 'header', + 'progress', + 'TEST-UNEXPECTED-FAIL', + 'trailer', + ]); + expect(lines[2]).toBe('TEST-UNEXPECTED-FAIL'); + }); +}); diff --git a/tests/ui/logviewer/useLogViewer_test.js b/tests/ui/logviewer/useLogViewer_test.js index 183986d58a3..48694304be4 100644 --- a/tests/ui/logviewer/useLogViewer_test.js +++ b/tests/ui/logviewer/useLogViewer_test.js @@ -46,6 +46,31 @@ describe('useLogViewer', () => { expect(result.current.error).toBeNull(); }); + test('splits bare CR and CRLF so line numbers match the backend parser', async () => { + // Logs commonly mix bare carriage returns (progress output) with CRLF. + // The backend records error line_numbers via bytes.splitlines(), which + // breaks on \r, \r\n and \n. The viewer must split identically or the + // red error highlight drifts off the real failure line. + global.fetch.mockResolvedValue({ + ok: true, + text: () => + Promise.resolve('header\rprogress\r\nTEST-UNEXPECTED-FAIL\ntrailer'), + }); + + const { result } = renderHook(() => useLogViewer({ url: 'http://log.txt' })); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + expect(result.current.lines).toEqual([ + 'header', + 'progress', + 'TEST-UNEXPECTED-FAIL', + 'trailer', + ]); + // Backend would report this failure at 0-indexed line_number 2; the viewer + // displays it at the matching array index 2 (display line 3). + expect(result.current.lines[2]).toBe('TEST-UNEXPECTED-FAIL'); + }); + test('sets error on fetch failure', async () => { global.fetch.mockResolvedValue({ ok: false, diff --git a/ui/logviewer/logviewerHelpers.js b/ui/logviewer/logviewerHelpers.js index 1eb9b1f4ba7..1ee8fb890de 100644 --- a/ui/logviewer/logviewerHelpers.js +++ b/ui/logviewer/logviewerHelpers.js @@ -1,5 +1,21 @@ import { getUrlParam, setUrlParam } from '../helpers/location'; +/** + * Split raw log text into display lines using the SAME line-boundary rules the + * backend log parser uses, so displayed line numbers line up with the error + * `line_number`s the backend records. + * + * The parser counts lines via `requests.iter_lines()` (Python's + * `bytes.splitlines()`), which breaks on `\n`, `\r`, AND `\r\n`. A plain + * `text.split('\n')` misses bare carriage returns — common in test progress + * output — so every line after a `\r` drifts, and the red error highlight lands + * a few lines off from the actual failure line. Splitting on the same set of + * separators keeps the viewer aligned with the parser. Handling `\r\n` as one + * break (rather than splitting on `\n` alone) also drops the stray trailing + * `\r` that CRLF logs would otherwise leave on every line. + */ +export const splitLogIntoLines = (text) => text.split(/\r\n|\r|\n/); + /** * Read the lineNumber URL param as an array of one or two ints, or null. */ diff --git a/ui/logviewer/useLogViewer.js b/ui/logviewer/useLogViewer.js index 3f1a81b169c..ae0d310d467 100644 --- a/ui/logviewer/useLogViewer.js +++ b/ui/logviewer/useLogViewer.js @@ -1,5 +1,7 @@ import { useState, useEffect, useCallback, useRef, useMemo } from 'react'; +import { splitLogIntoLines } from './logviewerHelpers'; + /** * Core hook for log viewing. Handles fetching, parsing, search, line selection, * scroll, and copy operations. @@ -61,7 +63,7 @@ export function useLogViewer({ }) .then((text) => { if (cancelled) return; - const parsed = text.split('\n'); + const parsed = splitLogIntoLines(text); setLines(parsed); setIsLoading(false); })