Skip to content

feat: implement test suites on computed metrics#16

Merged
GregoryAlbouy merged 26 commits intomainfrom
poc/test-suite
Aug 3, 2022
Merged

feat: implement test suites on computed metrics#16
GregoryAlbouy merged 26 commits intomainfrom
poc/test-suite

Conversation

@GregoryAlbouy
Copy link
Member

@GregoryAlbouy GregoryAlbouy commented Jul 27, 2022

Description

Some exploration regarding specs and implementation of formal test suite runs, without the use of template.

Specs

Features

  • Read a test config from any config file (json, yaml)
  • Read a test config from incoming HTTP request (json)
  • Run a test suite from a test config input
  • Output test results to CLI when run via CLI
  • Output JSON test response when run via server endpoint
  • Allow tests for any metric type
  • User-friendly error handling
    • Missing config field (name, field, predicate, target are required)
    • Unsupported value for field
    • Unsupported value for predicate
    • Incompatible value for target regarding the chosen field

Error handling

Error messages for each situation Capture d’écran 2022-08-01 à 12 34 25

CLI run

Config Output
request:
  url: https://example.com
runner:
  requests: 5
  concurrency: 1
tests:
  - name: minimum response time
    field: MIN
    predicate: GT
    target: 80ms
  - name: maximum response time
    field: MAX
    predicate: LTE
    target: 120ms
  - name: 100% availability
    field: FAILURE_COUNT
    predicate: EQ
    target: 0
benchttp-cli-output

Server run

Config Output
{
  "request": {
    "url": "https://example.com"
  },
  "runner": {
    "requests": 5,
    "concurrency": 1
  },
  "tests": [
    {
      "name": "minimum response time",
      "field": "MIN",
      "predicate": "GT",
      "target": "80ms"
    },
    {
      "name": "maximum response time",
      "field": "MAX",
      "predicate": "LTE",
      "target": "120ms"
    },
    {
      "name": "100% availability",
      "field": "FAILURE_COUNT",
      "predicate": "EQ",
      "target": "0"
    },
  ]
}
{
  "Metrics": {},
  "Metadata": {},
  "Tests": {
    "Pass": false,
    "Results": [
      {
        "Input": {
          "Name": "minimum response time",
          "Field": "MIN",
          "Predicate": "GT",
          "Target": 80000000
        },
        "Pass": true,
        "Summary": "want MIN > 80ms, got 83.392463ms"
      },
      {
        "Input": {
          "Name": "maximum response time",
          "Field": "MAX",
          "Predicate": "LTE",
          "Target": 120000000
        },
        "Pass": false,
        "Summary": "want MAX <= 120ms, got 433.415742ms"
      },
      {
        "Input": {
          "Name": "100% availability",
          "Field": "FAILURE_COUNT",
          "Predicate": "EQ",
          "Target": 0
        },
        "Pass": true,
        "Summary": "want FAILURE_COUNT == 0, got 0"
      }
    ]
  }
}

Demo

benchttp-test-suite-cli

Changes

Linked issues

Notes

@GregoryAlbouy GregoryAlbouy added this to the Next milestone Jul 27, 2022
@GregoryAlbouy GregoryAlbouy added must have Blocking feature for the targeted release scope:core scope:server scope:cli labels Jul 29, 2022
@GregoryAlbouy GregoryAlbouy marked this pull request as draft July 29, 2022 22:01
@GregoryAlbouy GregoryAlbouy marked this pull request as ready for review July 31, 2022 14:25
@GregoryAlbouy GregoryAlbouy changed the title poc: test suite feat: implement test suites on computed metrics Jul 31, 2022
@GregoryAlbouy GregoryAlbouy removed the poc label Jul 31, 2022
Copy link
Member

@moreirathomas moreirathomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice! It offers a great solution to the feature request 🙂

Question: do we continue supporting template outputs?

If it was used to answer the need for testing capabilities, this probably is a better replacement?

"strings"
"time"

"github.com/benchttp/engine/internal/cli/ansi"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: I'm not sure about this dependency.

runner/internal/... depends on internal/cli/....

Yet internal/cli depends on runner.

It is ok because it is different packages. Still I find it strange because as ansi is nested in cli.

We may argue that it is strange that a core package (report) has io/visualization concerns.

Although, not deal breaker at all for the pr.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may argue that it is strange that a core package (report) has io/visualization concerns.

I agree, this is the root of the problem to me. It's ok having a Report.String but it should not be specific to a CLI with ansi codes.

Probably a good candidate for refactoring in #26

@GregoryAlbouy GregoryAlbouy force-pushed the poc/test-suite branch 2 times, most recently from 0f37ce8 to fe3f93c Compare August 1, 2022 00:51
moreirathomas
moreirathomas previously approved these changes Aug 1, 2022
Copy link
Member

@moreirathomas moreirathomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great refactoring since last viewed 👍

- create package runner/internal/tests
- add result to server reponse (auto)
- display summary in CLI
- delegate metrics comparison logics to package metrics
- remove usage of metric getter
- use tests.Case as config.Global.Tests input
- adapt TestPredicate to new tests API
- several renamings:
  - metrics.Metric -> metrics.Source
  - tests.Input -> tests.Case
  - tests.SingleResult -> tests.CaseResult
- e.g. durations: "85675992" -> "85.675992ms"
- PassValues and FailValues were inverted for comparison cases
Before: `a.Compare(b) == compare(a, b)`
  `0.Compare(1) == compare(0, 1) == SUP`
Now: `a.Compare(b) == compare(a, b)`
  `0.Compare(1) == compare(1, 0) == INF`
The new order makes more sense because `a` remains the main interest
of the operation: when calling `a.Compare(b)` we want to get information
about `a` (when compared to `b`)
- remove helper metricGetter
- some renamings
- add documenting comments
- implement errorutil.WithDetails
- replace local implementations
- for field tests[n].target, JSON did not accept int values
  (but YAML did as they were read as string)
@GregoryAlbouy GregoryAlbouy merged commit bb6d64f into main Aug 3, 2022
@moreirathomas moreirathomas deleted the poc/test-suite branch August 19, 2022 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

must have Blocking feature for the targeted release scope:cli scope:core scope:server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate: test suites Implement test suites

2 participants