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: run snapshot tests dynamically in unimarkup-core crate #105

Merged
merged 14 commits into from
Sep 30, 2023

Conversation

nfejzic
Copy link
Collaborator

@nfejzic nfejzic commented Sep 9, 2023

This PR adds dynamic testing of snapshots in unimarkup_core crate.

By dynamic is meant that all spec files are automatically collected, parsed in order to generate test cases, and these test cases are run. Snapshot files are automatically created, and each test case (where each spec yaml file can contain multiple tests) is reported in cargo test output with corresponding name (and module path).

Downside of using custom harness is that using #[test] attribute to automatically test functions does not work (as far as I know). Any custom function must be manually collected. See test_fn! macro and how it's used.

Closes #100.

@nfejzic nfejzic requested a review from mhatzl September 9, 2023 14:16
@nfejzic nfejzic marked this pull request as draft September 9, 2023 14:37
@nfejzic nfejzic removed the request for review from mhatzl September 9, 2023 14:37
@nfejzic nfejzic requested a review from mhatzl September 9, 2023 14:52
@nfejzic nfejzic marked this pull request as ready for review September 9, 2023 14:52
@nfejzic nfejzic removed the request for review from mhatzl September 9, 2023 14:53
Copy link
Contributor

@mhatzl mhatzl left a comment

Choose a reason for hiding this comment

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

Thanks for this PR.

The blocks.rs file in core seems to be missing the snapshot test runner.
At the moment, only spec tests are run if I am correct.

Everything else looks ready to be merged.

Cargo.toml Show resolved Hide resolved
core/tests/blocks.rs Outdated Show resolved Hide resolved
core/tests/blocks.rs Outdated Show resolved Hide resolved
core/tests/blocks.rs Outdated Show resolved Hide resolved
core/tests/runner/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@mhatzl mhatzl left a comment

Choose a reason for hiding this comment

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

Looks good now.
Just not sure about case2 as variable name.

But I can change that myself once I merge #106 with these changes.
There will be some merge conflicts anyways, because of the Scanner change.

Copy link
Contributor

@mhatzl mhatzl left a comment

Choose a reason for hiding this comment

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

thanks 😊
looks good now 👍

@mhatzl mhatzl merged commit 94148b3 into main Sep 30, 2023
4 checks passed
@nfejzic nfejzic deleted the dynamic-testing branch October 2, 2023 07:52
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.

[REQ] Dynamic test implementation
2 participants