Proyecto hilo: loganalyzer (CLI de análisis de logs) - #6
Merged
Conversation
2 tasks
Guía para revisoresAñade el nuevo paquete instalable de Python Diagrama de secuencia para el punto de entrada CLI de loganalyzersequenceDiagram
actor User
participant CLI as cli_main
participant Parser as parse_file
participant Filters as filter_functions
participant Reporter as summarize_format_summary
User->>CLI: main(argv)
CLI->>CLI: _build_parser()
CLI->>CLI: parser.parse_args(argv)
CLI->>CLI: Path.exists()
alt fichero no existe
CLI-->>User: stderr Fichero no encontrado
CLI-->>User: return 1
else fichero existe
CLI->>Parser: parse_file(fichero)
Note over CLI,Filters: entries es un iterador lazy
opt level proporcionado
CLI->>Filters: filter_by_level(entries, level)
Filters-->>CLI: entries_filtrados
end
opt match proporcionado
CLI->>Filters: filter_by_pattern(entries, pattern)
Filters-->>CLI: entries_filtrados
end
CLI->>Reporter: summarize(entries, top_n)
Reporter-->>CLI: Summary
CLI->>Reporter: format_summary(Summary)
Reporter-->>CLI: texto_resumen
CLI-->>User: imprime resumen
CLI-->>User: return 0
end
Cambios a nivel de archivos
Consejos y comandosInteracción con Sourcery
Personalizar tu experienciaAccede a tu panel de control para:
Obtener ayuda
Original review guide in EnglishReviewer's GuideAdds the new installable Python package Sequence diagram for the loganalyzer CLI entrypointsequenceDiagram
actor User
participant CLI as cli_main
participant Parser as parse_file
participant Filters as filter_functions
participant Reporter as summarize_format_summary
User->>CLI: main(argv)
CLI->>CLI: _build_parser()
CLI->>CLI: parser.parse_args(argv)
CLI->>CLI: Path.exists()
alt fichero no existe
CLI-->>User: stderr Fichero no encontrado
CLI-->>User: return 1
else fichero existe
CLI->>Parser: parse_file(fichero)
Note over CLI,Filters: entries es un iterador lazy
opt level proporcionado
CLI->>Filters: filter_by_level(entries, level)
Filters-->>CLI: entries_filtrados
end
opt match proporcionado
CLI->>Filters: filter_by_pattern(entries, pattern)
Filters-->>CLI: entries_filtrados
end
CLI->>Reporter: summarize(entries, top_n)
Reporter-->>CLI: Summary
CLI->>Reporter: format_summary(Summary)
Reporter-->>CLI: texto_resumen
CLI-->>User: imprime resumen
CLI-->>User: return 0
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Hey, he encontrado 4 problemas y he dejado algunos comentarios de alto nivel:
- En
summarize, convertir todo el iterableentriesen una lista va en contra del diseño perezoso y por streaming deparse_filey de los filtros; plantéate calcular los contadores en una sola pasada sobre el iterador para mantener el uso de memoria acotado en logs grandes. - En
cli.main, la variableentriesse puede tipar simplemente comoIterable[LogEntry]en lugar deIterable[LogEntry] | Iterator[LogEntry], ya queIteratores un subtipo deIterabley la unión no aporta valor.
Prompt para agentes de IA
Por favor, atiende a los comentarios de esta revisión de código:
## Comentarios generales- En `summarize`, convertir todo el iterable `entries` en una lista va en contra del diseño perezoso y por streaming de `parse_file` y de los filtros; plantéate calcular los contadores en una sola pasada sobre el iterador para mantener el uso de memoria acotado en logs grandes.
- En `cli.main`, la variable `entries` se puede tipar simplemente como `Iterable[LogEntry]` en lugar de `Iterable[LogEntry] | Iterator[LogEntry]`, ya que `Iterator` es un subtipo de `Iterable` y la unión no aporta valor.
## Comentarios individuales### Comentario 1
<locationpath="proyecto/src/loganalyzer/reporter.py"line_range="22-23" />
<code_context>
+
+def summarize(entries: Iterable[LogEntry], top_n: int = 5) -> Summary:
+ """Calcula el resumen agregado de las entradas."""
+ entries_list = list(entries)
+ return Summary(
+ total=len(entries_list),+ by_level=dict(Counter(e.level for e in entries_list)),+ by_source=dict(Counter(e.source for e in entries_list)),+ top_messages=Counter(e.message for e in entries_list).most_common(top_n),+ )
+
</code_context>
<issue_to_address>
**suggestion (performance):** Evita materializar todas las entradas en una lista para reducir el uso de memoria y mejorar la escalabilidad.
Construir `entries_list` fuerza a cargar en memoria todo el stream de logs, lo que no escala bien para entradas grandes. En su lugar, itera una sola vez sobre `entries` y mantén `Counter`s y un total acumulado sobre la marcha, y luego deriva `top_messages` a partir de `by_message.most_common(top_n)`. Esto mantiene la memoria proporcional al número de claves distintas en lugar del número total de entradas.
```suggestiondef summarize(entries: Iterable[LogEntry], top_n: int = 5) -> Summary: """Calcula el resumen agregado de las entradas sin materializar todo el stream.""" by_level: Counter[str] = Counter() by_source: Counter[str] = Counter() by_message: Counter[str] = Counter() total = 0 for entry in entries: total += 1 by_level[entry.level] += 1 by_source[entry.source] += 1 by_message[entry.message] += 1 return Summary( total=total, by_level=dict(by_level), by_source=dict(by_source), top_messages=by_message.most_common(top_n), )```
</issue_to_address>
### Comentario 2
<locationpath="proyecto/src/loganalyzer/cli.py"line_range="51-52" />
<code_context>
+ entries: Iterable[LogEntry] | Iterator[LogEntry] = parse_file(args.fichero)
+ if args.level:
+ entries = filter_by_level(entries, args.level)+ if args.match:
+ entries = filter_by_pattern(entries, args.match)++ summary = summarize(entries, top_n=args.top)
</code_context>
<issue_to_address>
**issue (bug_risk):** Gestiona los patrones regex no válidos de forma elegante para evitar que la CLI se bloquee con un traceback.
Como `pattern` proviene de la entrada del usuario, `re.compile` en `filter_by_pattern` puede lanzar `re.error` para expresiones no válidas, lo que actualmente se propaga y muestra un stack trace. Plantéate capturar `re.error` alrededor de la llamada a `filter_by_pattern` en `main` y salir con un mensaje de error claro y un código de estado distinto de cero.
</issue_to_address>
### Comentario 3
<locationpath="proyecto/src/loganalyzer/parser.py"line_range="49-52" />
<code_context>
+ Lectura lazy: cada línea se procesa al vuelo, sin cargar el fichero entero
+ en memoria.
+ """
+ with open(ruta, encoding="utf-8") as f:
+ for linea in f:+ entry = parse_line(linea)+ if entry is not None:+ yield entry
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Plantéate manejar errores de E/S al abrir el fichero de logs para producir mensajes de error más claros en la CLI.
Dado que `parse_file` se llama desde la CLI, los `OSError` sin manejar que se produzcan en `open` actualmente se muestran como un stack trace. Plantéate capturarlos en el punto de llamada y devolver un error claro y un código de salida distinto de cero, por ejemplo:
```pythontry:
entries = parse_file(args.fichero)
exceptOSErroras exc:
print(f"No se pudo leer el fichero {args.fichero}: {exc}", file=sys.stderr)
return1```
Esto complementa la comprobación de existencia actual y mejora la robustez.
</issue_to_address>
### Comentario 4
<locationpath="proyecto/tests/test_cli.py"line_range="24-31" />
<code_context>
+ return path
++
+def test_main_runs_and_prints_summary(
+ sample_log: Path, capsys: pytest.CaptureFixture[str]
+) -> None:
+ exit_code = main([str(sample_log)])
+ assert exit_code == 0
+ output = capsys.readouterr().out
+# 3 válidas, 1 descartada por formato+ assert "Total de entradas: 3" in output
++
</code_context>
<issue_to_address>
**suggestion (testing):** Añade tests de CLI para la opción `--top` y para combinaciones de filtros / resultados vacíos.
Los tests de CLI actuales cubren la ejecución básica y los filtros más comunes, pero todavía no ejercitan `--top` ni los escenarios con resultados vacíos. Por favor, añade (1) un test que invoque `--top` con una distribución conocida de mensajes repetidos y compruebe que el número de líneas bajo `Top N mensajes:` coincide con el valor solicitado, y (2) un test en el que los filtros no devuelvan coincidencias (por ejemplo, `--level CRITICAL` sobre un log sin entradas CRITICAL) y verifique que el resumen imprime `Total de entradas: 0`. Esto confirmará que `top` se pasa correctamente a `summarize` y que la CLI gestiona correctamente los casos sin coincidencias.
```suggestiondef test_main_runs_and_prints_summary( sample_log: Path, capsys: pytest.CaptureFixture[str]) -> None: exit_code = main([str(sample_log)]) assert exit_code == 0 output = capsys.readouterr().out # 3 válidas, 1 descartada por formato assert "Total de entradas: 3" in outputdef test_main_top_flag_limits_number_of_top_messages( sample_log: Path, capsys: pytest.CaptureFixture[str]) -> None: exit_code = main([str(sample_log), "--top", "2"]) assert exit_code == 0 output = capsys.readouterr().out lines = output.splitlines() # Encontrar la línea de cabecera del top N header_index = None for i, line in enumerate(lines): if "Top 2 mensajes" in line: header_index = i break assert header_index is not None, "No se encontró la cabecera de 'Top 2 mensajes' en la salida del CLI" # Contar las líneas no vacías siguientes hasta el próximo bloque (línea en blanco) top_lines_count = 0 for line in lines[header_index + 1 :]: if not line.strip(): break top_lines_count += 1 assert top_lines_count == 2def test_main_with_filters_yielding_no_matches_prints_zero_entries( sample_log: Path, capsys: pytest.CaptureFixture[str]) -> None: # No hay entradas CRITICAL en el log de ejemplo, así que los filtros no deberían devolver nada exit_code = main([str(sample_log), "--level", "CRITICAL"]) assert exit_code == 0 output = capsys.readouterr().out assert "Total de entradas: 0" in output```
</issue_to_address>Sourcery es gratis para open source: si te gustan nuestras revisiones, por favor piensa en compartirlas ✨
Original comment in English
Hey - I've found 4 issues, and left some high level feedback:
- In
summarize, converting the entireentriesiterable to a list defeats the lazy, streaming design ofparse_fileand the filters; consider computing the counters in a single pass over the iterator to keep memory usage bounded for large logs. - In
cli.main, theentriesvariable can simply be typed asIterable[LogEntry]instead ofIterable[LogEntry] | Iterator[LogEntry], sinceIteratoris a subtype ofIterableand the union doesn’t add value.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- In `summarize`, converting the entire `entries` iterable to a list defeats the lazy, streaming design of `parse_file` and the filters; consider computing the counters in a single pass over the iterator to keep memory usage bounded for large logs.
- In `cli.main`, the `entries` variable can simply be typed as `Iterable[LogEntry]` instead of `Iterable[LogEntry] | Iterator[LogEntry]`, since `Iterator` is a subtype of `Iterable` and the union doesn’t add value.
## Individual Comments### Comment 1
<locationpath="proyecto/src/loganalyzer/reporter.py"line_range="22-23" />
<code_context>
+
+def summarize(entries: Iterable[LogEntry], top_n: int = 5) -> Summary:
+ """Calcula el resumen agregado de las entradas."""
+ entries_list = list(entries)
+ return Summary(
+ total=len(entries_list),+ by_level=dict(Counter(e.level for e in entries_list)),+ by_source=dict(Counter(e.source for e in entries_list)),+ top_messages=Counter(e.message for e in entries_list).most_common(top_n),+ )
+
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid materializing all entries into a list to reduce memory usage and improve scalability.
Building `entries_list` forces the whole log stream into memory, which doesn’t scale for large inputs. Instead, iterate once over `entries` and maintain `Counter`s and a running total as you go, then derive `top_messages` from `by_message.most_common(top_n)`. This keeps memory proportional to the number of distinct keys rather than the total number of entries.
```suggestiondef summarize(entries: Iterable[LogEntry], top_n: int = 5) -> Summary: """Calcula el resumen agregado de las entradas sin materializar todo el stream.""" by_level: Counter[str] = Counter() by_source: Counter[str] = Counter() by_message: Counter[str] = Counter() total = 0 for entry in entries: total += 1 by_level[entry.level] += 1 by_source[entry.source] += 1 by_message[entry.message] += 1 return Summary( total=total, by_level=dict(by_level), by_source=dict(by_source), top_messages=by_message.most_common(top_n), )```
</issue_to_address>
### Comment 2
<locationpath="proyecto/src/loganalyzer/cli.py"line_range="51-52" />
<code_context>
+ entries: Iterable[LogEntry] | Iterator[LogEntry] = parse_file(args.fichero)
+ if args.level:
+ entries = filter_by_level(entries, args.level)+ if args.match:
+ entries = filter_by_pattern(entries, args.match)++ summary = summarize(entries, top_n=args.top)
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle invalid regex patterns gracefully to avoid crashing the CLI with a traceback.
Because `pattern` comes from user input, `re.compile` in `filter_by_pattern` can raise `re.error` for invalid expressions, which currently bubbles up and prints a stack trace. Consider catching `re.error` around the `filter_by_pattern` call in `main` and exiting with a clear error message and non-zero status instead.
</issue_to_address>
### Comment 3
<locationpath="proyecto/src/loganalyzer/parser.py"line_range="49-52" />
<code_context>
+ Lectura lazy: cada línea se procesa al vuelo, sin cargar el fichero entero
+ en memoria.
+ """
+ with open(ruta, encoding="utf-8") as f:
+ for linea in f:+ entry = parse_line(linea)+ if entry is not None:+ yield entry
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider handling I/O errors when opening the log file to produce clearer CLI error messages.
Since `parse_file` is called from the CLI, unhandled `OSError`s from `open` will currently surface as a stack trace. Consider catching them at the call site and returning a clear error and non-zero exit code, e.g.:
```pythontry:
entries = parse_file(args.fichero)
exceptOSErroras exc:
print(f"No se pudo leer el fichero {args.fichero}: {exc}", file=sys.stderr)
return1```
This complements the existing existence check and improves robustness.
</issue_to_address>
### Comment 4
<locationpath="proyecto/tests/test_cli.py"line_range="24-31" />
<code_context>
+ return path
++
+def test_main_runs_and_prints_summary(
+ sample_log: Path, capsys: pytest.CaptureFixture[str]
+) -> None:
+ exit_code = main([str(sample_log)])
+ assert exit_code == 0
+ output = capsys.readouterr().out
+# 3 válidas, 1 descartada por formato+ assert "Total de entradas: 3" in output
++
</code_context>
<issue_to_address>
**suggestion (testing):** Add CLI tests for the `--top` flag and for combinations of filters / empty results.
Current CLI tests cover basic execution and common filters, but they don’t yet exercise `--top` or empty-result scenarios. Please add (1) a test invoking `--top` with a known distribution of repeated messages and assert the number of lines under `Top N mensajes:` matches the requested value, and (2) a test where filters yield zero matches (e.g., `--level CRITICAL` on a log without CRITICAL entries) and assert the summary prints `Total de entradas: 0`. This will verify that `top` is correctly passed to `summarize` and that the CLI handles no-match cases correctly.
```suggestiondef test_main_runs_and_prints_summary( sample_log: Path, capsys: pytest.CaptureFixture[str]) -> None: exit_code = main([str(sample_log)]) assert exit_code == 0 output = capsys.readouterr().out # 3 válidas, 1 descartada por formato assert "Total de entradas: 3" in outputdef test_main_top_flag_limits_number_of_top_messages( sample_log: Path, capsys: pytest.CaptureFixture[str]) -> None: exit_code = main([str(sample_log), "--top", "2"]) assert exit_code == 0 output = capsys.readouterr().out lines = output.splitlines() # Encontrar la línea de cabecera del top N header_index = None for i, line in enumerate(lines): if "Top 2 mensajes" in line: header_index = i break assert header_index is not None, "No se encontró la cabecera de 'Top 2 mensajes' en la salida del CLI" # Contar las líneas no vacías siguientes hasta el próximo bloque (línea en blanco) top_lines_count = 0 for line in lines[header_index + 1 :]: if not line.strip(): break top_lines_count += 1 assert top_lines_count == 2def test_main_with_filters_yielding_no_matches_prints_zero_entries( sample_log: Path, capsys: pytest.CaptureFixture[str]) -> None: # No hay entradas CRITICAL en el log de ejemplo, así que los filtros no deberían devolver nada exit_code = main([str(sample_log), "--level", "CRITICAL"]) assert exit_code == 0 output = capsys.readouterr().out assert "Total de entradas: 0" in output```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+22
to
+23
| def summarize(entries: Iterable[LogEntry], top_n: int = 5) -> Summary: | ||
| """Calcula el resumen agregado de las entradas.""" |
There was a problem hiding this comment.
suggestion (performance): Evita materializar todas las entradas en una lista para reducir el uso de memoria y mejorar la escalabilidad.
Construir entries_list fuerza a cargar en memoria todo el stream de logs, lo que no escala bien para entradas grandes. En su lugar, itera una sola vez sobre entries y mantén Counters y un total acumulado sobre la marcha, y luego deriva top_messages a partir de by_message.most_common(top_n). Esto mantiene la memoria proporcional al número de claves distintas en lugar del número total de entradas.
Suggested change
| defsummarize(entries: Iterable[LogEntry], top_n: int=5) ->Summary: | |
| """Calcula el resumen agregado de las entradas.""" | |
| defsummarize(entries: Iterable[LogEntry], top_n: int=5) ->Summary: | |
| """Calcula el resumen agregado de las entradas sin materializar todo el stream.""" | |
| by_level: Counter[str] =Counter() | |
| by_source: Counter[str] =Counter() | |
| by_message: Counter[str] =Counter() | |
| total=0 | |
| forentryinentries: | |
| total+=1 | |
| by_level[entry.level] +=1 | |
| by_source[entry.source] +=1 | |
| by_message[entry.message] +=1 | |
| returnSummary( | |
| total=total, | |
| by_level=dict(by_level), | |
| by_source=dict(by_source), | |
| top_messages=by_message.most_common(top_n), | |
| ) |
Original comment in English
suggestion (performance): Avoid materializing all entries into a list to reduce memory usage and improve scalability.
Building entries_list forces the whole log stream into memory, which doesn’t scale for large inputs. Instead, iterate once over entries and maintain Counters and a running total as you go, then derive top_messages from by_message.most_common(top_n). This keeps memory proportional to the number of distinct keys rather than the total number of entries.
Suggested change
| defsummarize(entries: Iterable[LogEntry], top_n: int=5) ->Summary: | |
| """Calcula el resumen agregado de las entradas.""" | |
| defsummarize(entries: Iterable[LogEntry], top_n: int=5) ->Summary: | |
| """Calcula el resumen agregado de las entradas sin materializar todo el stream.""" | |
| by_level: Counter[str] =Counter() | |
| by_source: Counter[str] =Counter() | |
| by_message: Counter[str] =Counter() | |
| total=0 | |
| forentryinentries: | |
| total+=1 | |
| by_level[entry.level] +=1 | |
| by_source[entry.source] +=1 | |
| by_message[entry.message] +=1 | |
| returnSummary( | |
| total=total, | |
| by_level=dict(by_level), | |
| by_source=dict(by_source), | |
| top_messages=by_message.most_common(top_n), | |
| ) |
Comment on lines
+51
to
+52
| if args.match: | ||
| entries = filter_by_pattern(entries, args.match) |
There was a problem hiding this comment.
issue (bug_risk): Gestiona los patrones regex no válidos de forma elegante para evitar que la CLI se bloquee con un traceback.
Como pattern proviene de la entrada del usuario, re.compile en filter_by_pattern puede lanzar re.error para expresiones no válidas, lo que actualmente se propaga y muestra un stack trace. Plantéate capturar re.error alrededor de la llamada a filter_by_pattern en main y salir con un mensaje de error claro y un código de estado distinto de cero.
Original comment in English
issue (bug_risk): Handle invalid regex patterns gracefully to avoid crashing the CLI with a traceback.
Because pattern comes from user input, re.compile in filter_by_pattern can raise re.error for invalid expressions, which currently bubbles up and prints a stack trace. Consider catching re.error around the filter_by_pattern call in main and exiting with a clear error message and non-zero status instead.
Comment on lines
+49
to
+52
| with open(ruta, encoding="utf-8") as f: | ||
| for linea in f: | ||
| entry = parse_line(linea) | ||
| if entry is not None: |
There was a problem hiding this comment.
suggestion (bug_risk): Plantéate manejar errores de E/S al abrir el fichero de logs para producir mensajes de error más claros en la CLI.
Dado que parse_file se llama desde la CLI, los OSError sin manejar que se produzcan en open actualmente se muestran como un stack trace. Plantéate capturarlos en el punto de llamada y devolver un error claro y un código de salida distinto de cero, por ejemplo:
try:
entries=parse_file(args.fichero)
exceptOSErrorasexc:
print(f"No se pudo leer el fichero {args.fichero}: {exc}", file=sys.stderr)
return1Esto complementa la comprobación de existencia actual y mejora la robustez.
Original comment in English
suggestion (bug_risk): Consider handling I/O errors when opening the log file to produce clearer CLI error messages.
Since parse_file is called from the CLI, unhandled OSErrors from open will currently surface as a stack trace. Consider catching them at the call site and returning a clear error and non-zero exit code, e.g.:
try:
entries=parse_file(args.fichero)
exceptOSErrorasexc:
print(f"No se pudo leer el fichero {args.fichero}: {exc}", file=sys.stderr)
return1This complements the existing existence check and improves robustness.
Comment on lines
+24
to
+31
| def test_main_runs_and_prints_summary( | ||
| sample_log: Path, capsys: pytest.CaptureFixture[str] | ||
| ) -> None: | ||
| exit_code = main([str(sample_log)]) | ||
| assert exit_code == 0 | ||
| output = capsys.readouterr().out | ||
| # 3 válidas, 1 descartada por formato | ||
| assert "Total de entradas: 3" in output |
There was a problem hiding this comment.
suggestion (testing): Añade tests de CLI para la opción --top y para combinaciones de filtros / resultados vacíos.
Los tests de CLI actuales cubren la ejecución básica y los filtros más comunes, pero todavía no ejercitan --top ni los escenarios con resultados vacíos. Por favor, añade (1) un test que invoque --top con una distribución conocida de mensajes repetidos y compruebe que el número de líneas bajo Top N mensajes: coincide con el valor solicitado, y (2) un test en el que los filtros no devuelvan coincidencias (por ejemplo, --level CRITICAL sobre un log sin entradas CRITICAL) y verifique que el resumen imprime Total de entradas: 0. Esto confirmará que top se pasa correctamente a summarize y que la CLI gestiona correctamente los casos sin coincidencias.
Suggested change
| deftest_main_runs_and_prints_summary( | |
| sample_log: Path, capsys: pytest.CaptureFixture[str] | |
| ) ->None: | |
| exit_code=main([str(sample_log)]) | |
| assertexit_code==0 | |
| output=capsys.readouterr().out | |
| # 3 válidas, 1 descartada por formato | |
| assert"Total de entradas: 3"inoutput | |
| deftest_main_runs_and_prints_summary( | |
| sample_log: Path, capsys: pytest.CaptureFixture[str] | |
| ) ->None: | |
| exit_code=main([str(sample_log)]) | |
| assertexit_code==0 | |
| output=capsys.readouterr().out | |
| # 3 válidas, 1 descartada por formato | |
| assert"Total de entradas: 3"inoutput | |
| deftest_main_top_flag_limits_number_of_top_messages( | |
| sample_log: Path, capsys: pytest.CaptureFixture[str] | |
| ) ->None: | |
| exit_code=main([str(sample_log), "--top", "2"]) | |
| assertexit_code==0 | |
| output=capsys.readouterr().out | |
| lines=output.splitlines() | |
| # Encontrar la línea de cabecera del top N | |
| header_index=None | |
| fori, lineinenumerate(lines): | |
| if"Top 2 mensajes"inline: | |
| header_index=i | |
| break | |
| assertheader_indexisnotNone, "No se encontró la cabecera de 'Top 2 mensajes' en la salida del CLI" | |
| # Contar las líneas no vacías siguientes hasta el próximo bloque (línea en blanco) | |
| top_lines_count=0 | |
| forlineinlines[header_index+1 :]: | |
| ifnotline.strip(): | |
| break | |
| top_lines_count+=1 | |
| asserttop_lines_count==2 | |
| deftest_main_with_filters_yielding_no_matches_prints_zero_entries( | |
| sample_log: Path, capsys: pytest.CaptureFixture[str] | |
| ) ->None: | |
| # No hay entradas CRITICAL en el log de ejemplo, así que los filtros no deberían devolver nada | |
| exit_code=main([str(sample_log), "--level", "CRITICAL"]) | |
| assertexit_code==0 | |
| output=capsys.readouterr().out | |
| assert"Total de entradas: 0"inoutput |
Original comment in English
suggestion (testing): Add CLI tests for the --top flag and for combinations of filters / empty results.
Current CLI tests cover basic execution and common filters, but they don’t yet exercise --top or empty-result scenarios. Please add (1) a test invoking --top with a known distribution of repeated messages and assert the number of lines under Top N mensajes: matches the requested value, and (2) a test where filters yield zero matches (e.g., --level CRITICAL on a log without CRITICAL entries) and assert the summary prints Total de entradas: 0. This will verify that top is correctly passed to summarize and that the CLI handles no-match cases correctly.
Suggested change
| deftest_main_runs_and_prints_summary( | |
| sample_log: Path, capsys: pytest.CaptureFixture[str] | |
| ) ->None: | |
| exit_code=main([str(sample_log)]) | |
| assertexit_code==0 | |
| output=capsys.readouterr().out | |
| # 3 válidas, 1 descartada por formato | |
| assert"Total de entradas: 3"inoutput | |
| deftest_main_runs_and_prints_summary( | |
| sample_log: Path, capsys: pytest.CaptureFixture[str] | |
| ) ->None: | |
| exit_code=main([str(sample_log)]) | |
| assertexit_code==0 | |
| output=capsys.readouterr().out | |
| # 3 válidas, 1 descartada por formato | |
| assert"Total de entradas: 3"inoutput | |
| deftest_main_top_flag_limits_number_of_top_messages( | |
| sample_log: Path, capsys: pytest.CaptureFixture[str] | |
| ) ->None: | |
| exit_code=main([str(sample_log), "--top", "2"]) | |
| assertexit_code==0 | |
| output=capsys.readouterr().out | |
| lines=output.splitlines() | |
| # Encontrar la línea de cabecera del top N | |
| header_index=None | |
| fori, lineinenumerate(lines): | |
| if"Top 2 mensajes"inline: | |
| header_index=i | |
| break | |
| assertheader_indexisnotNone, "No se encontró la cabecera de 'Top 2 mensajes' en la salida del CLI" | |
| # Contar las líneas no vacías siguientes hasta el próximo bloque (línea en blanco) | |
| top_lines_count=0 | |
| forlineinlines[header_index+1 :]: | |
| ifnotline.strip(): | |
| break | |
| top_lines_count+=1 | |
| asserttop_lines_count==2 | |
| deftest_main_with_filters_yielding_no_matches_prints_zero_entries( | |
| sample_log: Path, capsys: pytest.CaptureFixture[str] | |
| ) ->None: | |
| # No hay entradas CRITICAL en el log de ejemplo, así que los filtros no deberían devolver nada | |
| exit_code=main([str(sample_log), "--level", "CRITICAL"]) | |
| assertexit_code==0 | |
| output=capsys.readouterr().out | |
| assert"Total de entradas: 0"inoutput |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resumen
Bloque 3 del rediseño: añade el proyecto hilo
loganalyzercomo paquete Python instalable bajoproyecto/. Es la pieza que da continuidad al curso — cada módulo aporta una funcionalidad concreta sobre este proyecto.Cambios
proyecto/pyproject.toml: paquete instalable conhatchling, entry pointloganalyzer, deps de dev (pytest,ruff).proyecto/src/loganalyzer/:__init__.py: API pública.parser.py:LogEntry(dataclass frozen) +parse_line+parse_file(lectura lazy).filters.py:filter_by_level(severidad) +filter_by_pattern(regex).reporter.py:Summary+summarize(Counter por nivel/fuente/top mensajes) +format_summary.cli.py: punto de entrada conargparse. Flags--level,--match,--top.proyecto/tests/: 4 ficheros de test cubriendo parser, filters, reporter y CLI con fixturestmp_path/capsys.proyecto/samples/app.log: log sintético para probar la CLI sin tener que generar uno.proyecto/README.md: documentación de uso, estructura y roadmap por módulo del curso.Decisiones
argparse,re,dataclasses,collections). Las únicas deps son de dev (pytest, ruff).parse_fileconyield— permite analizar logs grandes sin cargar a memoria.Summaryesdataclass(frozen=True)para que sea hashable e inmutable.Plan de test
cd proyecto && uv sync --group dev && uv run pytest -v→ todos los tests verdes.uv run loganalyzer samples/app.log→ muestra el resumen.uv run loganalyzer samples/app.log --level ERROR→ filtra al nivel.uv run loganalyzer samples/app.log --match postgres→ filtra por regex.