Integration tests infrastructure

[jondot]

Built some infra for mainly integration tests, so I could have a good development experience.
Did the UUID builtin, to test the waters.

Looking for thoughts?

Workflow

Test:

$ cargo install cargo-insta
$ make build-opa
$ cargo test

Development:

• Drop a new rego policy in the fixture folder
• Drop a new json file for it, one or more
• Run a build make build-opa so that a new bundle would appear in fixtures
• Add a test pragma tuple to the TESTS array, (<bundle name>, <json input test case>), prefer a common prefix.

    ("test-loader", "test-loader.true"),

Run tests:

$ cargo test

Implement your code to pass the rego and json combo.
Follow insta to review failures

Principles

• Minimum effort writing new tests. Most tests should be just declaring the name and placing files in folders.
• Use insta to have a minimal effort for verifying tests, as well as enjoy the insta toolchain + serialization benefits
• Build bundles using the Go opa, but be less opinionated about how it gets to the fixtures folder (right now it's just a shell script and a makefile which can be replaced easily with more sophisticated mechanisms such as storing / caching it for speed)
• Each test starts from scratch, loading the wasm policy again and again: stateless and parallelizable

[sandhose]

A few comments here and there, but overall I like this a lot! Thanks for putting that together :)

I really would like to see the UUID implementation in another PR (possibly on top of that one for the integration tests) so I can properly review them independently

[jondot]

Thanks for the comments, I had a few 🤦 moments and also a learning experience :)
I'll dive into that in the coming days

[sandhose]

Just a heads-up that I did two things recently:

• I've setup GitHub Actions: Setup GitHub Actions #5
• I've gated builtins which adds dependencies behind cargo --features: 0c8f6eb

[jondot]

Just a heads-up that I did two things recently:

• I've setup GitHub Actions: Setup GitHub Actions #5
• I've gated builtins which adds dependencies behind cargo --features: 0c8f6eb

Perfect, thanks for the heads up!

[sandhose]

One thing I just realised, is that we usually require a license header in each file, and I need you to signoff your PR.
I have not written down the contribution guide yet, so I know it's not mentioned anywhere yet, but I have to apply the same policy we have on other Matrix.org projects. See https://matrix-org.github.io/synapse/latest/development/contributing_guide.html#sign-off for more info.

Could you please:

• add the licensing header to all .rego and .rs files you added?
• sign-off your PR, by including Signed-off-by: Your Name <[email protected]> in either your PR text or in all commits (git lets you do that automatically with git commit -s, or git rebase --signoff if you want to rewrite earlier commits)

Thanks a lot!

[jondot]

Thanks for the note, I'll be sure to do that, no problem 👍

[jondot]

Just updating that I've allocated some free time to work on this, I'll probably finish it up in a few days.

[jondot]

Done with this PR 🎊
Had to fight with making a nicer git history + signing off, but happy to see if there's something to improve there

[codecov-commenter]

Codecov Report

Merging #4 (47ea977) into main (4c6da4c) will increase coverage by 30.68%.
The diff coverage is 92.42%.

@@            Coverage Diff             @@
##            main       #4       +/-   ##
==========================================
+ Coverage   2.36%   33.05%   +30.68%     
==========================================
  Files         31       32        +1     
  Lines       1478     1543       +65     
==========================================
+ Hits          35      510      +475     
+ Misses      1443     1033      -410     

Impacted Files	Coverage Δ
src/builtins/impls/uuid.rs	0.00% <0.00%> (ø)
tests/smoke_test.rs	100.00% <100.00%> (ø)
src/policy.rs	54.31% <0.00%> (+54.31%)	⬆️
src/funcs.rs	58.07% <0.00%> (+58.07%)	⬆️
src/types.rs	65.47% <0.00%> (+65.47%)	⬆️
src/loader.rs	100.00% <0.00%> (+100.00%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c6da4c...47ea977. Read the comment docs.

[sandhose]

Some nitpicks, but other than that, LGTM! 👍

[sandhose]

Nice work, thanks a lot for all this! I'll let the CI run and merge once it passes.

I will try introduce an "evaluation context" which is carried throughout the evaluation, where builtins can grab a RNG and a cache

[jondot]

Thanks! -- it's been a learning experience, and I grabbed a few tricks from you here and there in my day to day work 😃