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

Add rstest for testing #32

Merged
merged 1 commit into from
Aug 15, 2022
Merged

Add rstest for testing #32

merged 1 commit into from
Aug 15, 2022

Conversation

trueleo
Copy link
Contributor

@trueleo trueleo commented Aug 14, 2022

Description

This PR introduces rstest as main unit testing framework. It's added as a development dependency to the project. maplit is added for its hashmap macro.

Core advantages of using rstest

  • can save on some boilerplate code
  • easier to add new test cases
  • #[once] Fixture to make an object initialise once and be shared between all tests

Disadvatages of using rstest

  • Complex struct initialisation might not be possible in a test case. This can still be a no-issue because we can always write normal rust tests for anything that can't be done using rstest.

What is changed

  • tests added to metadata.rs
  • tests converted in query.rs
  • tests converted in utils.rs

This PR has:

  • been tested to ensure log ingestion and log query works.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added documentation for new or modified features or behaviours.

@trueleo trueleo changed the title Add tests Add rstest for testing Aug 14, 2022
@nitisht nitisht requested a review from de-sh August 15, 2022 03:20
@nitisht
Copy link
Member

nitisht commented Aug 15, 2022

@de-sh any comments on the rstest use?

@nitisht nitisht merged commit 1e76c91 into parseablehq:main Aug 15, 2022
@trueleo trueleo deleted the add_tests branch August 15, 2022 10:00
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