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: export tests results as JSON #3785

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

notV4l
Copy link
Contributor

@notV4l notV4l commented Jul 30, 2023

Add an --export parameter to cairo-test accepting a path to export tests results as JSON.

use :

cairo-test ./src/tests.cairo --single-file --export ./tests.json

example results for Success / Fail :

[
  {
    "name": "tests::tests::test_1",
    "status": "Success",
    "gas_usage": 500
  },
 {
    "name": "tests::tests::test_2",
    "status": "Fail",
    "gas_usage": 1600,
    "error_messages": [
      "this is panic !!!",
      "with",
      "a long",
      "message"
    ]
  }
]

This change is Reviewable

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @notV4l)

a discussion (no related file):
please add more information directly into TestsSummary and add an export function to it directly.
I believe this would lead to much simpler code.


Copy link
Collaborator

@mkaput mkaput left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @notV4l and @orizi)

a discussion (no related file):
Blocking this because I do very much support this initiative, but I have big objections to the overall design of this feature.

  • This is rolling custom schema instead of using universally accepted standard formats like JUnit XML output, or TeamCity Messages.
  • One IMHO important use-case is omitted here: using in interactive test running UIs etc. like the test runner windows in IntelliJ and VSCode. For this to work, we need to stream messages to standard output instead of writing a single report to a file.

While it is completely possible to implement streaming output separately, I don't think it's good idea to have too many options? After all, streaming output can be easily piped to a file via shell, so it'd be functionally a superset.


@notV4l
Copy link
Contributor Author

notV4l commented Jul 31, 2023

I don't feel able to make modifs to make it work the way @mkaput described.
I could make a PR to fit @orizi requested changes (just let me know) but you maybe want to rework/implement this feature the way @mkaput suggested.
Anyway it could be convenient to have a simple JSON file as export option.

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

it does sound like @mkaput is correct - we should probably open this as an issue for contribution if you don't feel capable of doing it.

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @mkaput and @notV4l)

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.

3 participants