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.

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

feat: cli create test command #609

Merged
merged 26 commits into from
Jun 2, 2022
Merged

feat: cli create test command #609

merged 26 commits into from
Jun 2, 2022

Conversation

mathnogueira
Copy link
Member

@mathnogueira mathnogueira commented Jun 1, 2022

This PR adds the command tracetest test run --definition file.yml. Right now it just creates the test if it doesn't exist yet. I'll open more PRs to add the functionality for waiting until the result is finished and to update an existing test.

Other than the run command itself, it also introduces the text format specified in #160.

Loom video explaining it: https://www.loom.com/share/f738c2c1ef8e4500a358b76fa7ad1e63

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

@@ -23,7 +23,6 @@ components:
$ref: "#/components/schemas/HTTPHeader"
body:
type: string
format: byte
Copy link
Member Author

Choose a reason for hiding this comment

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

Basically, this means the body should be in base64 format. But this is not true. I removed this so prism could add the correct validations to the requests I'm sending.

Copy link
Collaborator

Choose a reason for hiding this comment

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

good catch

@mathnogueira mathnogueira requested a review from schoren June 1, 2022 20:46
- http.status = 200
- selector: span[span.name = "consume message from queue"]:last span[span.name = "save pokemon on database"]
assertions:
- db.repository.operation = "create"
Copy link
Member Author

Choose a reason for hiding this comment

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

There are some details in the syntax here. When defining an assertion, the value must be quoted if it is a string. Right now we support int, double, and string values.

@mathnogueira mathnogueira marked this pull request as ready for review June 1, 2022 20:50
@@ -0,0 +1,33 @@
name: POST import pokemon
description: Import a pokemon using its ID
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we have a description in the UI?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we don't. We have this field in the openapi spec though. We probably can remove it.

Copy link
Collaborator

@schoren schoren left a comment

Choose a reason for hiding this comment

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

Looking good! really nice code. I left some very minor nit comments, but this is ready to go!

type: raw
raw: '{ "id": 52 }'
testDefinition:
- selector: span[span.name = "POST /pokemon/import"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really like this format, super clear and concise

ShouldSucceed bool
}{
{
Name: "Should parse valid definition file",
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: when you get a fail message form go test, it replaces spaces with _. I like to use names that I can also search all files for. In this case, the go test error would look like TestLoadDefinition/Should_parse_valid_definition_file, and if I search for that, I won't have a match

func LoadDefinition(file string) (definition.Test, error) {
fileBytes, err := os.ReadFile(file)
if err != nil {
return definition.Test{}, fmt.Errorf("could not read file %s: %w", file, err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: a more specific message could help debugging, like could not read test definition file...

}

value := assertionParserObject.Value
if value[0:1] == "\"" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe this looks easier to read?

Suggested change
if value[0:1] == "\"" {
if value[0:1] == `"` {

@mathnogueira
Copy link
Member Author

Applied all changes! Thanks for the review @schoren

@mathnogueira mathnogueira merged commit 753f6bb into main Jun 2, 2022
@mathnogueira mathnogueira deleted the feat/cli-create-test branch June 2, 2022 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants