Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

catch2 time units are ignored when graphing and generating alerts #122

Open
paradust7 opened this issue May 5, 2022 · 1 comment · May be fixed by #239
Open

catch2 time units are ignored when graphing and generating alerts #122

paradust7 opened this issue May 5, 2022 · 1 comment · May be fixed by #239
Assignees
@ktrz
Labels
bug Something isn't working

Comments

@paradust7
Copy link

When using the "catch2" extractor, the time units, "ms", "us", etc, are extracted and printed, but otherwise ignored.

This results in broken graphs when the time is near a unit boundary:

broken_graph

It also results in false alerts:

Screenshot from 2022-05-04 17-17-24
@paradust7 paradust7 mentioned this issue May 5, 2022
Merged
@wwerkk
Copy link

wwerkk commented Apr 18, 2024

Any news on this? getRatio doesn't involve unit calculation atm, though it seems like an easy fix for a problem that makes this action pretty useless for catch:

github-action-benchmark/src/write.ts

Line 567 in 8e74972

function getRatio(tool: ToolType, prev: BenchmarkResult, current: BenchmarkResult) {

@wwerkk wwerkk linked a pull request Apr 18, 2024 that will close this issue
Open
@ktrz ktrz self-assigned this May 19, 2024
@ktrz ktrz added the bug Something isn't working label May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ktrz ktrz

Labels
bug Something isn't working
Projects
Benchmark Action Board
Status: In Progress
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Issue #122: Alerts unit bug fix wwerkk/github-action-benchmark
3 participants
@ktrz @wwerkk @paradust7