Repository: microsoft/regorus (language: python)

Issue:
Policy Coverage
Provide a way to figure out what lines of policy were evaluated.


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml
index 6aaf774d..a34582e6 100644
--- a/.github/workflows/pr.yml
+++ b/.github/workflows/pr.yml
@@ -33,19 +33,12 @@ jobs:
     - name: Run tests (OPA Conformance)
       run: >-
         cargo test -r --test opa --features opa-testutil  -- $(tr '\n' ' ' < tests/opa.passing)
-    - name: Run tests (coverage)
-      run: cargo test -r --verbose --features "coverage"
-    - name: Run tests (ACI coverage)
-      run: cargo test -r --test aci --features "coverage"
-    - name: Run tests (OPA Conformance Coverage)
-      run: >-
-        cargo test -r --test opa --features "opa-testutil,coverage"  -- $(tr '\n' ' ' < tests/opa.passing)
     - name: Build (MUSL)
       run: cargo build --verbose --all-targets --target x86_64-unknown-linux-musl
     - name: Run tests (MUSL)
       run: cargo test -r --verbose --target x86_64-unknown-linux-musl
     - name: Run tests (MUSL ACI)
-      run: cargo test -r --test aci --features "coverage" --target x86_64-unknown-linux-musl
-    - name: Run tests (MUSL OPA Conformance Coverage)
+      run: cargo test -r --test aci --target x86_64-unknown-linux-musl
+    - name: Run tests (MUSL OPA Conformance)
       run: >-
-        cargo test -r --test opa --features "opa-testutil,coverage" --target x86_64-unknown-linux-musl  -- $(tr '\n' ' ' < tests/opa.passing)
+        cargo test -r --test opa --features opa-testutil --target x86_64-unknown-linux-musl  -- $(tr '\n' ' ' < tests/opa.passing)
diff --git a/Cargo.toml b/Cargo.toml
index aaa7f1b8..406178c7 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -46,6 +46,7 @@ yaml = ["serde_yaml"]
 full-opa = [
     "base64",
     "base64url",
+    "coverage",
     "crypto",
     "deprecated",
     "glob",
diff --git a/README.md b/README.md
index 326321ba..b4f813fb 100644
--- a/README.md
+++ b/README.md
@@ -13,7 +13,7 @@ Regorus is also
   - *extensible* - Extend the Rego language by implementing custom stateful builtins in Rust.
     See [add_extension](https://github.com/microsoft/regorus/blob/fc68bf9c8bea36427dae9401a7d1f6ada771f7ab/src/engine.rs#L352).
     Support for extensibility using other languages coming soon.
-  - *polyglot* - In addition to Rust, Regorus can be used from *C*, *C++*, *C#*, *Golang*, *Javascript* and *Python*.
+  - *polyglot* - In addition to Rust, Regorus can be used from *C*, *C++*, *C#*, *Golang*, *Java*, *Javascript* and *Python*.
     This is made possible by the excellent FFI tools available in the Rust ecosystem. See [bindings](#bindings) for information on how to use Regorus from different languages.
 
     To try out a *Javascript(WASM)* compiled version of Regorus from your browser, visit [Regorus Playground](https://anakrish.github.io/regorus-playground/).
@@ -85,6 +85,8 @@ Regorus can be used from a variety of languages:
 - *C#*: C# binding is generated using [csbindgen](https://github.com/Cysharp/csbindgen). See [bindings/csharp](https://github.com/microsoft/regorus/tree/main/bindings/csharp) for an example of how to build and use Regorus in your C# projects.
 - *Golang*: The C bindings are exposed to Golang via [CGo](https://pkg.go.dev/cmd/cgo). See [bindings/go](https://github.com/microsoft/regorus/tree/main/bindings/go) for an example of how to build and use Regorus in your Go projects.
 - *Python*: Python bindings are generated using [pyo3](https://github.com/PyO3/pyo3). Wheels are created using [maturin](https://github.com/PyO3/maturin). See [bindings/python](https://github.com/microsoft/regorus/tree/main/bindings/python).
+- *Java*: Java bindings are developed using [jni-rs](https://github.com/jni-rs/jni-rs).
+  See [bindings/java](https://github.com/microsoft/regorus/tree/main/bindings/java).
 - *Javascript*: Regorus is compiled to WASM using [wasmpack](https://github.com/rustwasm/wasm-pack).
   See [bindings/wasm](https://github.com/microsoft/regorus/tree/main/bindings/wasm) for an example of using Regorus from nodejs.
   To try out a *Javascript(WASM)* compiled version of Regorus from your browser, visit [Regorus Playground](https://anakrish.github.io/regorus-playground/).
@@ -149,7 +151,7 @@ This produces the following output
 }
 ```
 
-Next, evaluate a sample [policy](examples/example.rego) and [input](examples/input.json)
+Next, evaluate a sample [policy](https://github.com/microsoft/regorus/blob/main/examples/example.rego) and [input](https://github.com/microsoft/regorus/blob/main/examples/input.json)
 (borrowed from [Rego tutorial](https://www.openpolicyagent.org/docs/latest/#2-try-opa-eval)):
 
 ```bash
@@ -162,6 +164,22 @@ Finally, evaluate real-world [policies](tests/aci/) used in Azure Container Inst
 $ regorus eval -b tests/aci -d tests/aci/data.json -i tests/aci/input.json data.policy.mount_overlay=x
 ```
 
+## Policy coverage
+
+Regorus allows determining which lines of a policy have been executed using the `coverage` feature (enabled by default).
+
+We can try it out using the `regorus` example program by passing in the `--coverage` flag.
+
+```shell
+$ regorus eval -d examples/example.rego -i examples/input.json data.example --coverage
+```
+
+It produces the following coverage report which shows that all lines are executed except the line that sets `allow` to true.
+![coverage.png](https://github.com/microsoft/regorus/blob/main/docs/coverage.png)
+
+See [Engine::get_coverage_report](https://docs.rs/regorus/latest/regorus/struct.Engine.html#method.get_coverage_report) for details.
+Policy coverage information is useful for debugging your policy as well as to write tests for your policy so that all 
+lines of the policy are exercised by the tests.
 
 ## ACI Policies
 
diff --git a/docs/coverage.png b/docs/coverage.png
new file mode 100644
index 00000000..ed28e58d
Binary files /dev/null and b/docs/coverage.png differ
diff --git a/examples/regorus.rs b/examples/regorus.rs
index 04d0b377..2adc048c 100644
--- a/examples/regorus.rs
+++ b/examples/regorus.rs
@@ -17,6 +17,9 @@ fn rego_eval(
 
     engine.set_strict_builtin_errors(!non_strict);
 
+    #[cfg(feature = "coverage")]
+    engine.set_enable_coverage(coverage);
+
     // Load files from given bundles.
     for dir in bundles.iter() {
         let entries =
@@ -73,24 +76,8 @@ fn rego_eval(
 
     #[cfg(feature = "coverage")]
     if coverage {
-        println!("\n\nCOVERAGE REPORT");
-        // Fetch coverage report.
         let report = engine.get_coverage_report()?;
-        for file in report.files.into_iter() {
-            if file.uncovered.is_empty() {
-                println!("{} has full coverage", file.path);
-                continue;
-            }
-
-            println!("{}:", file.path);
-            for (line, code) in file.code.split('\n').enumerate() {
-                if file.uncovered.contains(&(line as u32 + 1)) {
-                    println!("\x1b[31m {line:4}  {code}\x1b[0m");
-                } else {
-                    println!(" {line:4}  {code}");
-                }
-            }
-        }
+        println!("{}", report.to_colored_string()?);
     }
 
     Ok(())
diff --git a/scripts/pre-push b/scripts/pre-push
index fa232bdf..ba60d5d7 100755
--- a/scripts/pre-push
+++ b/scripts/pre-push
@@ -18,9 +18,4 @@ if [ -f Cargo.toml ]; then
 
    # Ensure that OPA conformance tests don't regress.
    cargo test -r --features opa-testutil --test opa -- $(tr '\n' ' ' < tests/opa.passing)
-
-   # Run the same tests using "coverage" build
-   cargo test -r --features "coverage"
-   cargo test -r --test aci --features "coverage"
-   cargo test -r --features "coverage,opa-testutil" --test opa -- $(tr '\n' ' ' < tests/opa.passing)
 fi
diff --git a/src/engine.rs b/src/engine.rs
index accc2872..f0c5c802 100644
--- a/src/engine.rs
+++ b/src/engine.rs
@@ -458,4 +458,14 @@ impl Engine {
     pub fn get_coverage_report(&self) -> Result<crate::coverage::Report> {
         self.interpreter.get_coverage_report()
     }
+
+    #[cfg(feature = "coverage")]
+    pub fn set_enable_coverage(&mut self, enable: bool) {
+        self.interpreter.set_enable_coverage(enable)
+    }
+
+    #[cfg(feature = "coverage")]
+    pub fn clear_coverage_data(&mut self) {
+        self.interpreter.clear_coverage_data()
+    }
 }
diff --git a/src/interpreter.rs b/src/interpreter.rs
index df30ee89..fce242da 100644
--- a/src/interpreter.rs
+++ b/src/interpreter.rs
@@ -69,6 +69,8 @@ pub struct Interpreter {
 
     #[cfg(feature = "coverage")]
     coverage: HashMap<Source, Vec<bool>>,
+    #[cfg(feature = "coverage")]
+    enable_coverage: bool,
 }
 
 impl Default for Interpreter {
@@ -183,6 +185,8 @@ impl Interpreter {
 
             #[cfg(feature = "coverage")]
             coverage: HashMap::new(),
+            #[cfg(feature = "coverage")]
+            enable_coverage: false,
         }
     }
 
@@ -2590,7 +2594,7 @@ impl Interpreter {
         );
 
         #[cfg(feature = "coverage")]
-        {
+        if self.enable_coverage {
             let span = expr.span();
             let source = &span.source;
             let line = span.line as usize;
@@ -3508,11 +3512,11 @@ impl Interpreter {
     }
 
     #[cfg(feature = "coverage")]
-    fn gather_uncovered_lines_in_query(
+    fn gather_coverage_in_query(
         &self,
         query: &Ref<Query>,
         covered: &Vec<bool>,
-        uncovered: &mut BTreeSet<u32>,
+        file: &mut crate::coverage::File,
     ) -> Result<()> {
         for stmt in &query.stmts {
             // TODO: with mods
@@ -3521,15 +3525,15 @@ impl Interpreter {
                 Literal::SomeIn {
                     value, collection, ..
                 } => {
-                    self.gather_uncovered_lines_in_expr(value, covered, uncovered)?;
-                    self.gather_uncovered_lines_in_expr(collection, covered, uncovered)?;
+                    self.gather_coverage_in_expr(value, covered, file)?;
+                    self.gather_coverage_in_expr(collection, covered, file)?;
                 }
                 Literal::Expr { expr, .. } | Literal::NotExpr { expr, .. } => {
-                    self.gather_uncovered_lines_in_expr(expr, covered, uncovered)?;
+                    self.gather_coverage_in_expr(expr, covered, file)?;
                 }
                 Literal::Every { domain, query, .. } => {
-                    self.gather_uncovered_lines_in_expr(domain, covered, uncovered)?;
-                    self.gather_uncovered_lines_in_query(query, covered, uncovered)?;
+                    self.gather_coverage_in_expr(domain, covered, file)?;
+                    self.gather_coverage_in_query(query, covered, file)?;
                 }
             }
         }
@@ -3537,23 +3541,25 @@ impl Interpreter {
     }
 
     #[cfg(feature = "coverage")]
-    fn gather_uncovered_lines_in_expr(
+    fn gather_coverage_in_expr(
         &self,
         expr: &Ref<Expr>,
         covered: &Vec<bool>,
-        uncovered: &mut BTreeSet<u32>,
+        file: &mut crate::coverage::File,
     ) -> Result<()> {
         use Expr::*;
         traverse(expr, &mut |e| {
             Ok(match e.as_ref() {
                 ArrayCompr { query, .. } | SetCompr { query, .. } | ObjectCompr { query, .. } => {
-                    self.gather_uncovered_lines_in_query(query, covered, uncovered)?;
+                    self.gather_coverage_in_query(query, covered, file)?;
                     false
                 }
                 _ => {
                     let line = e.span().line as usize;
                     if line >= covered.len() || !covered[line] {
-                        uncovered.insert(line as u32);
+                        file.not_covered.insert(line as u32);
+                    } else if line < covered.len() && covered[line] {
+                        file.covered.insert(line as u32);
                     }
                     true
                 }
@@ -3574,7 +3580,12 @@ impl Interpreter {
                 continue;
             };
 
-            let mut uncovered = BTreeSet::new();
+            let mut file = crate::coverage::File {
+                path: span.source.file().clone(),
+                code: span.source.contents().clone(),
+                covered: BTreeSet::new(),
+                not_covered: BTreeSet::new(),
+            };
 
             // Loop through each rule and figure out the lines that were not coverd.
             for rule in &module.policy {
@@ -3583,46 +3594,41 @@ impl Interpreter {
                         match head {
                             RuleHead::Compr { assign, .. } | RuleHead::Func { assign, .. } => {
                                 if let Some(a) = assign {
-                                    self.gather_uncovered_lines_in_expr(
-                                        &a.value,
-                                        covered,
-                                        &mut uncovered,
-                                    )?;
+                                    self.gather_coverage_in_expr(&a.value, covered, &mut file)?;
                                 }
                             }
                             RuleHead::Set { key, .. } => {
                                 if let Some(k) = key {
-                                    self.gather_uncovered_lines_in_expr(
-                                        k,
-                                        covered,
-                                        &mut uncovered,
-                                    )?;
+                                    self.gather_coverage_in_expr(k, covered, &mut file)?;
                                 }
                             }
                         }
                         for b in bodies {
-                            self.gather_uncovered_lines_in_query(
-                                &b.query,
-                                covered,
-                                &mut uncovered,
-                            )?;
+                            self.gather_coverage_in_query(&b.query, covered, &mut file)?;
                         }
                     }
                     Rule::Default { value, .. } => {
-                        self.gather_uncovered_lines_in_expr(value, covered, &mut uncovered)?;
+                        self.gather_coverage_in_expr(value, covered, &mut file)?;
                     }
                 }
             }
 
-            let file = crate::coverage::PolicyFile {
-                path: span.source.file().clone(),
-                code: span.source.contents().clone(),
-                uncovered,
-            };
-
             report.files.push(file);
         }
 
         Ok(report)
     }
+
+    #[cfg(feature = "coverage")]
+    pub fn set_enable_coverage(&mut self, enable: bool) {
+        if self.enable_coverage != enable {
+            self.enable_coverage = enable;
+            self.clear_coverage_data();
+        }
+    }
+
+    #[cfg(feature = "coverage")]
+    pub fn clear_coverage_data(&mut self) {
+        self.coverage = HashMap::new();
+    }
 }
diff --git a/src/lib.rs b/src/lib.rs
index ee4a9ead..bf754354 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -341,16 +341,46 @@ impl std::fmt::Debug for dyn Extension {
 
 #[cfg(feature = "coverage")]
 pub mod coverage {
-    #[derive(Default, serde::Serialize, serde::Deserialize, Eq, PartialEq)]
-    pub struct PolicyFile {
+    #[derive(Default, serde::Serialize, serde::Deserialize)]
+    pub struct File {
         pub path: String,
         pub code: String,
-        pub uncovered: std::collections::BTreeSet<u32>,
+        pub covered: std::collections::BTreeSet<u32>,
+        pub not_covered: std::collections::BTreeSet<u32>,
     }
 
-    #[derive(Default, serde::Serialize, serde::Deserialize, Eq, PartialEq)]
+    #[derive(Default, serde::Serialize, serde::Deserialize)]
     pub struct Report {
-        pub files: Vec<PolicyFile>,
+        pub files: Vec<File>,
+    }
+
+    impl Report {
+        pub fn to_colored_string(&self) -> anyhow::Result<String> {
+            use std::io::Write;
+            let mut s = Vec::new();
+            writeln!(&mut s, "COVERAGE REPORT:")?;
+            for file in self.files.iter() {
+                if file.not_covered.is_empty() {
+                    writeln!(&mut s, "{} has full coverage", file.path)?;
+                    continue;
+                }
+
+                writeln!(&mut s, "{}:", file.path)?;
+                for (line, code) in file.code.split('\n').enumerate() {
+                    let line = line as u32 + 1;
+                    if file.not_covered.contains(&line) {
+                        writeln!(&mut s, "\x1b[31m {line:4}  {code}\x1b[0m")?;
+                    } else if file.covered.contains(&line) {
+                        writeln!(&mut s, "\x1b[32m {line:4}  {code}\x1b[0m")?;
+                    } else {
+                        writeln!(&mut s, " {line:4}  {code}")?;
+                    }
+                }
+            }
+
+            writeln!(&mut s)?;
+            Ok(std::str::from_utf8(&s)?.to_string())
+        }
     }
 }
 


Test patch:
diff --git a/src/tests/interpreter/mod.rs b/src/tests/interpreter/mod.rs
index bd2d2d97..04b2c1d7 100644
--- a/src/tests/interpreter/mod.rs
+++ b/src/tests/interpreter/mod.rs
@@ -143,6 +143,9 @@ pub fn eval_file(
     let mut engine: Engine = Engine::new();
     engine.set_strict_builtin_errors(strict);
 
+    #[cfg(feature = "coverage")]
+    engine.set_enable_coverage(true);
+
     let mut results = vec![];
     let mut files = vec![];
 
diff --git a/tests/aci/main.rs b/tests/aci/main.rs
index f55dcbcc..038ca186 100644
--- a/tests/aci/main.rs
+++ b/tests/aci/main.rs
@@ -111,6 +111,7 @@ fn run_aci_tests(dir: &Path) -> Result<()> {
 #[cfg(feature = "coverage")]
 fn run_aci_tests_coverage(dir: &Path) -> Result<()> {
     let mut engine = Engine::new();
+    engine.set_enable_coverage(true);
 
     let mut added = std::collections::BTreeSet::new();
 
@@ -150,24 +151,8 @@ fn run_aci_tests_coverage(dir: &Path) -> Result<()> {
         }
     }
 
-    println!("\n\nCOVERAGE REPORT");
-    // Fetch coverage report.
     let report = engine.get_coverage_report()?;
-    for file in report.files.into_iter() {
-        if file.uncovered.is_empty() {
-            println!("{} has full coverage", file.path);
-            continue;
-        }
-
-        println!("{}:", file.path);
-        for (line, code) in file.code.split('\n').enumerate() {
-            if file.uncovered.contains(&(line as u32 + 1)) {
-                println!("\x1b[31m {line:4}  {code}\x1b[0m");
-            } else {
-                println!(" {line:4}  {code}");
-            }
-        }
-    }
+    println!("{}", report.to_colored_string()?);
 
     Ok(())
 }
@@ -184,11 +169,8 @@ struct Cli {
 fn main() -> Result<()> {
     let cli = Cli::parse();
 
-    cfg_if::cfg_if! {
-    if #[cfg(feature = "coverage")] {
-        run_aci_tests_coverage(&Path::new(&cli.test_dir))
-    } else {
-        run_aci_tests(&Path::new(&cli.test_dir))
-    }
-    }
+    #[cfg(feature = "coverage")]
+    run_aci_tests_coverage(&Path::new(&cli.test_dir))?;
+
+    run_aci_tests(&Path::new(&cli.test_dir))
 }
diff --git a/tests/coverage/mod.rs b/tests/coverage/mod.rs
index fa225faf..188e2741 100644
--- a/tests/coverage/mod.rs
+++ b/tests/coverage/mod.rs
@@ -8,6 +8,12 @@ use regorus::*;
 use anyhow::Result;
 use test_generator::test_resources;
 
+#[derive(serde::Deserialize)]
+struct File {
+    covered: BTreeSet<u32>,
+    not_covered: BTreeSet<u32>,
+}
+
 #[derive(serde::Deserialize)]
 struct TestCase {
     data: Option<Value>,
@@ -15,8 +21,8 @@ struct TestCase {
     modules: Vec<String>,
     note: String,
     query: String,
-    uncovered: Vec<BTreeSet<u32>>,
     skip: Option<bool>,
+    report: Vec<File>,
 }
 
 #[derive(serde::Deserialize)]
@@ -38,6 +44,7 @@ fn yaml_test_impl(file: &str) -> Result<()> {
         }
 
         let mut engine = Engine::new();
+        engine.set_enable_coverage(true);
 
         for (idx, rego) in case.modules.iter().enumerate() {
             engine.add_policy(format!("rego_{idx}"), rego.clone())?;
@@ -54,8 +61,9 @@ fn yaml_test_impl(file: &str) -> Result<()> {
         let _ = engine.eval_query(case.query.clone(), false)?;
         let report = engine.get_coverage_report()?;
 
-        for (idx, uncovered) in case.uncovered.into_iter().enumerate() {
-            assert_eq!(uncovered, report.files[idx].uncovered);
+        for (idx, file) in case.report.into_iter().enumerate() {
+            assert_eq!(file.not_covered, report.files[idx].not_covered);
+            assert_eq!(file.covered, report.files[idx].covered);
         }
 
         println!("passed");
diff --git a/tests/coverage/tests.yaml b/tests/coverage/tests.yaml
index a105c0ea..330fd7c4 100644
--- a/tests/coverage/tests.yaml
+++ b/tests/coverage/tests.yaml
@@ -14,7 +14,8 @@ cases:
           k = input.k
         }
     query: data.test
-    uncovered: [
-      [ 5, 7 ]
-    ]
+    report:
+      - covered: [3, 6]
+        not_covered: [5, 7]
+
         
diff --git a/tests/opa.rs b/tests/opa.rs
index 874b6852..fbb24683 100644
--- a/tests/opa.rs
+++ b/tests/opa.rs
@@ -54,6 +54,9 @@ struct YamlTest {
 fn eval_test_case(case: &TestCase) -> Result<Value> {
     let mut engine = Engine::new();
 
+    #[cfg(feature = "coverage")]
+    engine.set_enable_coverage(true);
+
     if let Some(data) = &case.data {
         engine.add_data(data.clone())?;
     }