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

convert most tests in tests/ to YAML ztests #342

Merged
merged 4 commits into from
Feb 21, 2020
Merged

convert most tests in tests/ to YAML ztests #342

merged 4 commits into from
Feb 21, 2020

Conversation

nwt
Copy link
Member

@nwt nwt commented Feb 20, 2020

  • Add package ztest to run formulaic tests defined in YAML files that
    look like this:
    zql: count()

    input: |
      #0:record[i:int64]
      0:[1;]
      0:[2;]

    output: |
      #0:record[count:uint64]
      0:[2;]
  • Convert tests in tests/formats and most of those in tests/suite to
    YAML. As a side effect, both "make test-unit" and "make test-system"
    now run all of the converted tests. (test-unit uses
    driver.(*Driver).Run, while test-system uses the dist/zq executable).

Future Work

  • Move YAML ztests into the packages they're intended to test. (I put them in the same directory as the tests they replaces to make this change easier to digest.)
  • Convert most of the tests in proc/ and some of those in filter/ and zio/* to YAML.
  • Convert the remaining tests in tests/suite/errors/. (This requires adding something like an "error expected" field to ztest.Ztest.)
  • Pick a directory in which to put regression tests.

* Add package ztest to run formulaic tests defined in YAML files that
  look like this:

    zql: count()

    input: |
      #0:record[i:int64]
      0:[1;]
      0:[2;]

    output: |
      #0:record[count:uint64]
      0:[2;]

* Convert tests in tests/formats and most of those in tests/suite to
  YAML.  As a side effect, both "make test-unit" and "make test-system"
  now run all of the converted tests.  (test-unit uses
  driver.(*Driver).Run, while test-system uses the dist/zq executable).
@nwt nwt requested a review from a team February 20, 2020 21:18
Copy link
Collaborator

@mattnibs mattnibs left a comment

Choose a reason for hiding this comment

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

Excellent!

@aswan
Copy link

aswan commented Feb 20, 2020

I'm not very familiar with yaml, is !!binary handled by the parser? For bzng data, it would be great if we could write the data out in hex rather than base64. If somebody is looking at a particular test (or diffs to a particular test) it is tedious to be sure, but feasible to read through the byte-by-byte representation of binary data. This isn't really practical with base64.

tests/ztest_test.go Show resolved Hide resolved
ztest/ztest.go Show resolved Hide resolved
ztest/ztest.go Show resolved Hide resolved
ztest/ztest.go Outdated Show resolved Hide resolved
tests/ztest_test.go Show resolved Hide resolved
@nwt
Copy link
Member Author

nwt commented Feb 21, 2020

@aswan: !!binary is handled by the parser. It is, unfortunately, the only means YAML defines for encoding non-UTF-8 byte sequences.

We could always define our own binary encoding above YAML. Do you think that's worth doing?

@aswan
Copy link

aswan commented Feb 21, 2020

We could always define our own binary encoding above YAML. Do you think that's worth doing?

Maybe? That could happen in a later follow-up for sure if it seems like it would be worthwhile.

ztest/ztest.go Outdated Show resolved Hide resolved
@mikesbrown mikesbrown self-requested a review February 21, 2020 19:43
@nwt nwt merged commit da56b01 into master Feb 21, 2020
@nwt nwt deleted the ztest branch February 21, 2020 22:59
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.

5 participants