From 76450012fe2d96e1eab4c70388511bfab53dd10e Mon Sep 17 00:00:00 2001 From: Kesha Hietala Date: Thu, 8 Feb 2024 16:59:55 -0500 Subject: [PATCH 01/11] pulling relevant edits from fse-timings branch --- cedar | 2 +- cedar-drt/fuzz/src/lib.rs | 8 +- cedar-drt/src/cedar_test_impl.rs | 100 -------------------- cedar-drt/src/dafny_java_impl.rs | 134 +++++++++------------------ cedar-drt/src/lean_impl.rs | 92 +++++++----------- cedar-drt/src/lib.rs | 2 - cedar-drt/tests/integration_tests.rs | 7 +- 7 files changed, 90 insertions(+), 255 deletions(-) delete mode 100644 cedar-drt/src/cedar_test_impl.rs diff --git a/cedar b/cedar index 7de733312..6838bee0f 160000 --- a/cedar +++ b/cedar @@ -1 +1 @@ -Subproject commit 7de7333126816772ffa7da46766e4f287c6d9c86 +Subproject commit 6838bee0f87567052aea0b201bfb87d61566839e diff --git a/cedar-drt/fuzz/src/lib.rs b/cedar-drt/fuzz/src/lib.rs index 044cde3e7..1a40312f3 100644 --- a/cedar-drt/fuzz/src/lib.rs +++ b/cedar-drt/fuzz/src/lib.rs @@ -20,9 +20,7 @@ mod prt; pub use dump::*; pub use prt::*; -use cedar_drt::{ - time_function, CedarTestImplementation, ErrorComparisonMode, RUST_AUTH_MSG, RUST_VALIDATION_MSG, -}; +use cedar_drt::{time_function, CedarTestImplementation, ErrorComparisonMode}; use cedar_policy::{frontend::is_authorized::InterfaceResponse, PolicyId}; use cedar_policy_core::ast; use cedar_policy_core::authorizer::{AuthorizationError, Authorizer, Response}; @@ -34,6 +32,10 @@ use libfuzzer_sys::arbitrary::{self, Unstructured}; use log::info; use std::collections::HashSet; +/// Times for cedar-policy authorization and validation. +pub const RUST_AUTH_MSG: &str = "rust_auth (ns) : "; +pub const RUST_VALIDATION_MSG: &str = "rust_validation (ns) : "; + /// Compare the behavior of the evaluator in `cedar-policy` against a custom Cedar /// implementation. Panics if the two do not agree. `expr` is the expression to /// evaluate and `request` and `entities` are used to populate the evaluator. diff --git a/cedar-drt/src/cedar_test_impl.rs b/cedar-drt/src/cedar_test_impl.rs deleted file mode 100644 index 00c2b1d30..000000000 --- a/cedar-drt/src/cedar_test_impl.rs +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright 2022-2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -//! Definition of a general `CedarTestImplementation` trait that describes an -//! implementation of Cedar to use during testing. - -use cedar_policy::frontend::is_authorized::InterfaceResponse; -pub use cedar_policy::Response; -use cedar_policy_core::ast::{Expr, PolicySet, Request, Value}; -pub use cedar_policy_core::*; -pub use cedar_policy_validator::{ValidationMode, ValidationResult, ValidatorSchema}; -pub use entities::Entities; -use serde::Deserialize; - -/// Type alias for convenience. Errors are represented as strings to make -/// (de)serialization as simple as possible. For an `InterfaceResult`, an -/// error represents a case where the external Cedar implementation failed -/// to execute the request (e.g., due to a parse error). -pub type InterfaceResult = std::result::Result; - -/// "Interface" type for `ValidationResult` which represents validation -/// errors as strings. -#[derive(Debug, Deserialize)] -pub struct InterfaceValidationResult { - #[serde(rename = "validationErrors")] - pub validation_errors: Vec, -} - -impl InterfaceValidationResult { - pub fn validation_passed(&self) -> bool { - self.validation_errors.is_empty() - } -} - -/// A custom implementation of the Cedar authorizer and validator used for testing. -pub trait CedarTestImplementation { - /// Custom authorizer entry point. - fn is_authorized( - &self, - request: Request, - policies: &PolicySet, - entities: &Entities, - ) -> InterfaceResult; - - /// Custom evaluator entry point. The bool return value indicates the whether - /// evaluating the provided expression produces the expected value. - /// `expected` is optional to allow for the case where no return value is - /// expected due to errors. - fn interpret( - &self, - request: Request, - entities: &Entities, - expr: &Expr, - expected: Option, - ) -> InterfaceResult; - - /// Custom validator entry point. - fn validate( - &self, - schema: &ValidatorSchema, - policies: &PolicySet, - mode: ValidationMode, - ) -> InterfaceResult; - - /// `ErrorComparisonMode` that should be used for this `CedarTestImplementation` - fn error_comparison_mode(&self) -> ErrorComparisonMode; -} - -/// Specifies how errors coming from a `CedarTestImplementation` should be -/// compared against errors coming from the Rust implementation. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub enum ErrorComparisonMode { - /// Don't compare errors at all; the `CedarTestImplementation` is not - /// expected to produce errors matching the Rust implementation's errors in - /// any way. - /// In fact, the `CedarTestImplementation` will be expected to never report - /// errors. - Ignore, - /// The `CedarTestImplementation` is expected to produce "error messages" that - /// are actually just the id of the erroring policy. This will be compared to - /// ensure that the `CedarTestImplementation` agrees with the Rust - /// implementation on which policies produce errors. - PolicyIds, - /// The `CedarTestImplementation` is expected to produce error messages that - /// exactly match the Rust implementation's error messages' `Display` text. - Full, -} diff --git a/cedar-drt/src/dafny_java_impl.rs b/cedar-drt/src/dafny_java_impl.rs index 9426d5208..6907eaffc 100644 --- a/cedar-drt/src/dafny_java_impl.rs +++ b/cedar-drt/src/dafny_java_impl.rs @@ -17,11 +17,9 @@ //! Implementation of the [`CedarTestImplementation`] trait for the Cedar Java //! implementation extracted from the Dafny specification. -use crate::cedar_test_impl::*; use crate::definitional_request_types::*; -use crate::logger::*; +use cedar_policy::cedar_test_impl::*; use cedar_policy::frontend::is_authorized::InterfaceResponse; -use cedar_policy::integration_testing::{CustomCedarImpl, IntegrationTestValidationResult}; use cedar_policy_core::ast::{Expr, Value}; pub use cedar_policy_core::*; pub use cedar_policy_validator::{ValidationMode, ValidatorSchema}; @@ -31,20 +29,9 @@ use jni::{JNIVersion, JavaVM}; use lazy_static::lazy_static; use log::info; use serde::Deserialize; +use std::collections::HashMap; -/// Times to (de)serialize JSON content sent to / received from the Dafny-Java -/// implementation. -pub const RUST_SERIALIZATION_MSG: &str = "rust_serialization (ns) : "; -pub const RUST_DESERIALIZATION_MSG: &str = "rust_deserialization (ns) : "; - -/// Times for cedar-policy authorization and validation. -pub const RUST_AUTH_MSG: &str = "rust_auth (ns) : "; -pub const RUST_VALIDATION_MSG: &str = "rust_validation (ns) : "; - -/// Times for JSON (de)serialization, authorization, and validation as reported -/// by the Dafny-Java implementation. -pub const JAVA_SERIALIZATION_MSG: &str = "java_serialization (ns) : "; -pub const JAVA_DESERIALIZATION_MSG: &str = "java_deserialization (ns) : "; +/// Times for JSON authorization and validation as reported by the Dafny-Java implementation pub const JAVA_AUTH_MSG: &str = "java_auth (ns) : "; pub const JAVA_VALIDATION_MSG: &str = "java_validation (ns) : "; @@ -69,9 +56,9 @@ lazy_static! { #[derive(Debug, Deserialize)] pub struct AuthorizationResponse { - pub serialization_nanos: i64, - pub deserialization_nanos: i64, - pub auth_nanos: i64, + pub serialization_nanos: u128, + pub deserialization_nanos: u128, + pub auth_nanos: u128, pub response: InterfaceResponse, } @@ -91,9 +78,9 @@ pub struct ValidationResponseInner { #[derive(Debug, Deserialize)] pub struct ValidationResponse { - pub serialization_nanos: i64, - pub deserialization_nanos: i64, - pub validation_nanos: i64, + pub serialization_nanos: u128, + pub deserialization_nanos: u128, + pub validation_nanos: u128, pub response: ValidationResponseInner, } @@ -222,7 +209,7 @@ impl<'j> JavaDefinitionalEngine<'j> { .expect("failed to create Java object for authorization request string") } - fn deserialize_auth_response(&self, response: JValue) -> InterfaceResponse { + fn deserialize_auth_response(&self, response: JValue) -> TestResponse { let jresponse: JString = response .l() .unwrap_or_else(|_| { @@ -246,17 +233,16 @@ impl<'j> JavaDefinitionalEngine<'j> { ) }); - info!( - "{}{}", - JAVA_SERIALIZATION_MSG, d_response.serialization_nanos - ); - info!( - "{}{}", - JAVA_DESERIALIZATION_MSG, d_response.deserialization_nanos - ); info!("{}{}", JAVA_AUTH_MSG, d_response.auth_nanos); - d_response.response + let response = TestResponse { + response: d_response.response, + timing_info: HashMap::from([( + "authorize_and_parse".into(), + d_response.auth_nanos / 1000, + )]), + }; + response } /// Ask the definitional engine whether `isAuthorized` for the given `request`, @@ -266,10 +252,8 @@ impl<'j> JavaDefinitionalEngine<'j> { request: &ast::Request, policies: &ast::PolicySet, entities: &Entities, - ) -> InterfaceResponse { - let (jstring, dur) = - time_function(|| self.serialize_auth_request(request, policies, entities)); - info!("{}{}", RUST_SERIALIZATION_MSG, dur.as_nanos()); + ) -> TestResponse { + let jstring: JString<'_> = self.serialize_auth_request(request, policies, entities); let response = self.thread.call_method( self.def_authorizer, "isAuthorized_str", @@ -284,8 +268,7 @@ impl<'j> JavaDefinitionalEngine<'j> { panic!("JVM Exception Occurred!"); } let response: JValue = response.expect("failed to call Java isAuthorized_str"); - let (response, dur) = time_function(|| self.deserialize_auth_response(response)); - info!("{}{}", RUST_DESERIALIZATION_MSG, dur.as_nanos()); + let response = self.deserialize_auth_response(response); self.thread .delete_local_ref(*jstring) .expect("Deletion failed"); @@ -309,7 +292,7 @@ impl<'j> JavaDefinitionalEngine<'j> { .expect("failed to create Java object for validation request string") } - fn deserialize_val_response(&self, response: JValue) -> InterfaceValidationResult { + fn deserialize_val_response(&self, response: JValue) -> TestValidationResult { let jresponse: JString = response .l() .unwrap_or_else(|_| { @@ -334,14 +317,6 @@ impl<'j> JavaDefinitionalEngine<'j> { ) }); - info!( - "{}{}", - JAVA_SERIALIZATION_MSG, d_response.serialization_nanos - ); - info!( - "{}{}", - JAVA_DESERIALIZATION_MSG, d_response.deserialization_nanos - ); info!("{}{}", JAVA_VALIDATION_MSG, d_response.validation_nanos); assert_eq!( @@ -349,8 +324,12 @@ impl<'j> JavaDefinitionalEngine<'j> { Vec::::new(), "Dafny json parsing failed", ); - InterfaceValidationResult { - validation_errors: d_response.response.validation_errors, + TestValidationResult { + errors: d_response.response.validation_errors, + timing_info: HashMap::from([( + "validation_and_parse".into(), + d_response.validation_nanos / 1000, + )]), } } @@ -360,9 +339,8 @@ impl<'j> JavaDefinitionalEngine<'j> { schema: &ValidatorSchema, policies: &ast::PolicySet, mode: ValidationMode, - ) -> InterfaceValidationResult { - let (jstring, dur) = time_function(|| self.serialize_val_request(schema, policies, mode)); - info!("{}{}", RUST_SERIALIZATION_MSG, dur.as_nanos()); + ) -> TestValidationResult { + let jstring = self.serialize_val_request(schema, policies, mode); let response = self.thread.call_method( self.def_validator, "validate_str", @@ -377,8 +355,7 @@ impl<'j> JavaDefinitionalEngine<'j> { panic!("JVM Exception Occurred!"); } let response: JValue = response.expect("failed to call Java validate_str"); - let (response, dur) = time_function(|| self.deserialize_val_response(response)); - info!("{}{}", RUST_DESERIALIZATION_MSG, dur.as_nanos()); + let response = self.deserialize_val_response(response); self.thread .delete_local_ref(*jstring) .expect("Deletion failed"); @@ -389,21 +366,26 @@ impl<'j> JavaDefinitionalEngine<'j> { impl<'j> CedarTestImplementation for JavaDefinitionalEngine<'j> { fn is_authorized( &self, - request: ast::Request, + request: &ast::Request, policies: &ast::PolicySet, entities: &Entities, - ) -> InterfaceResult { - Ok(self.is_authorized(&request, policies, entities)) + ) -> TestResult { + TestResult::Success(self.is_authorized(request, policies, entities)) } fn interpret( &self, - request: ast::Request, + request: &ast::Request, entities: &Entities, expr: &Expr, + enable_extensions: bool, expected: Option, - ) -> InterfaceResult { - Ok(self.eval(&request, entities, expr, expected)) + ) -> TestResult { + assert!( + enable_extensions, + "Dafny definitional interpreter expects extensions to be enabled" + ); + TestResult::Success(self.eval(request, entities, expr, expected)) } fn validate( @@ -411,39 +393,11 @@ impl<'j> CedarTestImplementation for JavaDefinitionalEngine<'j> { schema: &cedar_policy_validator::ValidatorSchema, policies: &ast::PolicySet, mode: ValidationMode, - ) -> InterfaceResult { - Ok(self.validate(schema, policies, mode)) + ) -> TestResult { + TestResult::Success(self.validate(schema, policies, mode)) } fn error_comparison_mode(&self) -> ErrorComparisonMode { ErrorComparisonMode::Ignore } } - -/// Implementation of the trait used for integration testing. -impl<'j> CustomCedarImpl for JavaDefinitionalEngine<'j> { - fn is_authorized( - &self, - request: &ast::Request, - policies: &ast::PolicySet, - entities: &Entities, - ) -> InterfaceResponse { - self.is_authorized(request, policies, entities) - } - - fn validate( - &self, - schema: cedar_policy_validator::ValidatorSchema, - policies: &ast::PolicySet, - ) -> cedar_policy::integration_testing::IntegrationTestValidationResult { - let definitional_res = self.validate( - &schema, - policies, - cedar_policy_validator::ValidationMode::default(), - ); - IntegrationTestValidationResult { - validation_passed: definitional_res.validation_passed(), - validation_errors_debug: format!("{:?}", definitional_res.validation_errors), - } - } -} diff --git a/cedar-drt/src/lean_impl.rs b/cedar-drt/src/lean_impl.rs index 01ea47fce..c7f25ddc4 100644 --- a/cedar-drt/src/lean_impl.rs +++ b/cedar-drt/src/lean_impl.rs @@ -21,12 +21,11 @@ // we've already initialized. use core::panic; +use std::collections::HashMap; use std::{env, ffi::CString}; -use crate::cedar_test_impl::*; use crate::definitional_request_types::*; -use cedar_policy::frontend::is_authorized::InterfaceResponse; -use cedar_policy::integration_testing::{CustomCedarImpl, IntegrationTestValidationResult}; +use cedar_policy::cedar_test_impl::*; use cedar_policy_core::ast::{Expr, Value}; pub use cedar_policy_core::*; pub use cedar_policy_validator::{ValidationMode, ValidatorSchema}; @@ -80,7 +79,7 @@ enum ResultDef { #[derive(Debug, Deserialize)] struct TimedDef { data: T, - duration: i64, + duration: u128, } #[derive(Debug, Deserialize)] @@ -144,15 +143,14 @@ impl LeanDefinitionalEngine { unsafe { lean_mk_string(cstring.as_ptr() as *const u8) } } - fn deserialize_authorization_response( - response: *mut lean_object, - ) -> InterfaceResult { + fn deserialize_authorization_response(response: *mut lean_object) -> TestResult { let response_string = lean_obj_to_string(response); let resp: AuthorizationResponse = serde_json::from_str(&response_string).expect("could not deserialize json"); match resp { AuthorizationResponse::Ok(resp) => { - info!("{}{}", LEAN_AUTH_MSG, resp.duration); + let auth_time = resp.duration / 1000; // nanoseconds -> microseconds + info!("{LEAN_AUTH_MSG}{auth_time}"); let resp = resp.data; let decision: authorizer::Decision = match resp.decision.as_str() { @@ -181,9 +179,12 @@ impl LeanDefinitionalEngine { pid.to_string() }) .collect(); - Ok(InterfaceResponse::new(decision, reason, errors)) + TestResult::Success(TestResponse { + response: InterfaceResponse::new(decision, reason, errors), + timing_info: HashMap::from([("authorize".into(), auth_time)]), + }) } - AuthorizationResponse::Error(err) => Err(err), + AuthorizationResponse::Error(err) => TestResult::Failure(err), } } @@ -194,7 +195,7 @@ impl LeanDefinitionalEngine { request: &ast::Request, policies: &ast::PolicySet, entities: &Entities, - ) -> InterfaceResult { + ) -> TestResult { let req = Self::serialize_authorization_request(request, policies, entities); let response = unsafe { isAuthorizedDRT(req) }; Self::deserialize_authorization_response(response) @@ -217,16 +218,16 @@ impl LeanDefinitionalEngine { unsafe { lean_mk_string(cstring.as_ptr() as *const u8) } } - fn deserialize_evaluation_response(response: *mut lean_object) -> InterfaceResult { + fn deserialize_evaluation_response(response: *mut lean_object) -> TestResult { let response_string = lean_obj_to_string(response); let resp: EvaluationResponse = serde_json::from_str(&response_string).expect("could not deserialize json"); match resp { EvaluationResponse::Ok(resp) => { info!("{}{}", LEAN_EVAL_MSG, resp.duration); - Ok(resp.data) + TestResult::Success(resp.data) } - EvaluationResponse::Error(err) => Err(err), + EvaluationResponse::Error(err) => TestResult::Failure(err), } } @@ -238,7 +239,7 @@ impl LeanDefinitionalEngine { entities: &Entities, expr: &Expr, expected: Option, - ) -> InterfaceResult { + ) -> TestResult { let expected_as_expr = expected.map(|v| v.into()); let req = Self::serialize_evaluation_request(request, entities, expr, expected_as_expr.as_ref()); @@ -262,7 +263,7 @@ impl LeanDefinitionalEngine { fn deserialize_validation_response( response: *mut lean_object, - ) -> InterfaceResult { + ) -> TestResult { let response_string = lean_obj_to_string(response); let resp: ValidationResponse = serde_json::from_str(&response_string).expect("could not deserialize json"); @@ -273,9 +274,13 @@ impl LeanDefinitionalEngine { ValidationResponseInner::Ok(_) => Vec::new(), ValidationResponseInner::Error(err) => vec![err], }; - Ok(InterfaceValidationResult { validation_errors }) + let response = TestValidationResult { + errors: validation_errors, + timing_info: HashMap::from([("validation".into(), resp.duration / 1000)]), + }; + TestResult::Success(response) } - ValidationResponse::Error(err) => Err(err), + ValidationResponse::Error(err) => TestResult::Failure(err), } } @@ -284,7 +289,7 @@ impl LeanDefinitionalEngine { &self, schema: &ValidatorSchema, policies: &ast::PolicySet, - ) -> InterfaceResult { + ) -> TestResult { let req = Self::serialize_validation_request(schema, policies); let response = unsafe { validateDRT(req) }; Self::deserialize_validation_response(response) @@ -294,21 +299,26 @@ impl LeanDefinitionalEngine { impl CedarTestImplementation for LeanDefinitionalEngine { fn is_authorized( &self, - request: ast::Request, + request: &ast::Request, policies: &ast::PolicySet, entities: &Entities, - ) -> InterfaceResult { - self.is_authorized(&request, policies, entities) + ) -> TestResult { + self.is_authorized(request, policies, entities) } fn interpret( &self, - request: ast::Request, + request: &ast::Request, entities: &Entities, expr: &Expr, + enable_extensions: bool, expected: Option, - ) -> InterfaceResult { - self.evaluate(&request, entities, expr, expected) + ) -> TestResult { + assert!( + enable_extensions, + "Lean definitional interpreter expects extensions to be enabled" + ); + self.evaluate(request, entities, expr, expected) } fn validate( @@ -316,7 +326,7 @@ impl CedarTestImplementation for LeanDefinitionalEngine { schema: &cedar_policy_validator::ValidatorSchema, policies: &ast::PolicySet, mode: ValidationMode, - ) -> InterfaceResult { + ) -> TestResult { assert_eq!( mode, ValidationMode::Strict, @@ -329,33 +339,3 @@ impl CedarTestImplementation for LeanDefinitionalEngine { ErrorComparisonMode::PolicyIds } } - -/// Implementation of the trait used for integration testing. The integration -/// tests expect the calls to `is_authorized` and `validate` to succeed. -impl CustomCedarImpl for LeanDefinitionalEngine { - fn is_authorized( - &self, - request: &ast::Request, - policies: &ast::PolicySet, - entities: &Entities, - ) -> InterfaceResponse { - self.is_authorized(request, policies, entities) - .unwrap_or_else(|e| { - panic!("Unexpected error from the Lean implementation of `is_authorized`: {e}") - }) - } - - fn validate( - &self, - schema: cedar_policy_validator::ValidatorSchema, - policies: &ast::PolicySet, - ) -> IntegrationTestValidationResult { - let result = self.validate(&schema, policies).unwrap_or_else(|e| { - panic!("Unexpected error from the Lean implementation of `validate`: {e}") - }); - IntegrationTestValidationResult { - validation_passed: result.validation_passed(), - validation_errors_debug: format!("{:?}", result.validation_errors), - } - } -} diff --git a/cedar-drt/src/lib.rs b/cedar-drt/src/lib.rs index a1383dd82..088ea3e52 100644 --- a/cedar-drt/src/lib.rs +++ b/cedar-drt/src/lib.rs @@ -14,14 +14,12 @@ * limitations under the License. */ -mod cedar_test_impl; mod dafny_java_impl; mod definitional_request_types; mod lean_impl; mod logger; pub mod utils; -pub use cedar_test_impl::*; pub use dafny_java_impl::*; pub use definitional_request_types::*; pub use lean_impl::*; diff --git a/cedar-drt/tests/integration_tests.rs b/cedar-drt/tests/integration_tests.rs index 06d754f8d..d0430f20f 100644 --- a/cedar-drt/tests/integration_tests.rs +++ b/cedar-drt/tests/integration_tests.rs @@ -17,8 +17,9 @@ //! Integration test that runs the handwritten test cases from //! `cedar-integration-tests` on the definitional implementation. +use cedar_policy::cedar_test_impl::CedarTestImplementation; use cedar_policy::integration_testing::{ - perform_integration_test_from_json_custom, resolve_integration_test_path, CustomCedarImpl, + perform_integration_test_from_json_custom, resolve_integration_test_path, }; use cedar_drt::*; @@ -30,7 +31,7 @@ fn folder() -> &'static Path { Path::new("tests") } -fn run_integration_tests(custom_impl: &dyn CustomCedarImpl) { +fn run_integration_tests(test_impl: &impl CedarTestImplementation) { let integration_tests_folder = resolve_integration_test_path(folder()); // find all `*.json` files in the integration tests folder let test_jsons = WalkDir::new(&integration_tests_folder) @@ -47,7 +48,7 @@ fn run_integration_tests(custom_impl: &dyn CustomCedarImpl) { // `#[test]` fails, the user can know which test case failed by looking // for the last "Running integration test" message before the failure. println!("Running integration test: {:?}", test_json); - perform_integration_test_from_json_custom(&test_json, Some(custom_impl)); + perform_integration_test_from_json_custom(&test_json, test_impl); println!("Integration test succeeded: {:?}", test_json); } } From 4a9ebba9ee0be378d9210d2507eda68da4e07a59 Mon Sep 17 00:00:00 2001 From: Kesha Hietala Date: Sun, 11 Feb 2024 15:12:41 +0000 Subject: [PATCH 02/11] fix some build issues --- cedar-drt/fuzz/src/lib.rs | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/cedar-drt/fuzz/src/lib.rs b/cedar-drt/fuzz/src/lib.rs index 1a40312f3..bf794ec30 100644 --- a/cedar-drt/fuzz/src/lib.rs +++ b/cedar-drt/fuzz/src/lib.rs @@ -20,8 +20,11 @@ mod prt; pub use dump::*; pub use prt::*; -use cedar_drt::{time_function, CedarTestImplementation, ErrorComparisonMode}; -use cedar_policy::{frontend::is_authorized::InterfaceResponse, PolicyId}; +use cedar_policy::cedar_test_impl::{ + time_function, CedarTestImplementation, ErrorComparisonMode, TestResult, +}; +use cedar_policy::frontend::is_authorized::InterfaceResponse; +use cedar_policy::PolicyId; use cedar_policy_core::ast; use cedar_policy_core::authorizer::{AuthorizationError, Authorizer, Response}; use cedar_policy_core::entities::{Entities, NoEntitiesSchema, TCComputation}; @@ -64,11 +67,17 @@ pub fn run_eval_test( // `custom_impl.interpret()` returns true when the result of evaluating `expr` // matches `expected` - let definitional_res = custom_impl.interpret(request.clone(), entities, expr, expected.clone()); + let definitional_res = custom_impl.interpret( + &request, + entities, + expr, + enable_extensions, + expected.clone(), + ); // TODO(#175): For now, ignore cases where the definitional code returned an error due to // an unknown extension function. - if let Err(err) = definitional_res.clone() { + if let TestResult::Failure(err) = definitional_res { if err.contains("jsonToExtFun: unknown extension function") { return; } @@ -77,7 +86,7 @@ pub fn run_eval_test( // Otherwise, `definitional_res` should be `Ok(true)` assert_eq!( definitional_res, - Ok(true), + TestResult::Success(true), "Incorrect evaluation result for {request}\nExpression:\n{expr}\nEntities:\n{entities}\nExpected value:\n{:?}\n", expected ) @@ -108,10 +117,10 @@ pub fn run_auth_test( return rust_res; } - let definitional_res = custom_impl.is_authorized(request.clone(), policies, entities); + let definitional_res = custom_impl.is_authorized(&request, policies, entities); match definitional_res { - Err(err) => { + TestResult::Failure(err) => { // TODO(#175): For now, ignore cases where the Lean code returned an error due to // an unknown extension function. if err.contains("jsonToExtFun: unknown extension function") { @@ -123,7 +132,7 @@ pub fn run_auth_test( ); } } - Ok(definitional_res) => { + TestResult::Success(definitional_res) => { let rust_res_for_comparison: InterfaceResponse = { let errors = match custom_impl.error_comparison_mode() { ErrorComparisonMode::Ignore => HashSet::new(), @@ -190,7 +199,7 @@ pub fn run_val_test( if rust_res.validation_passed() { match definitional_res { - Err(err) => { + TestResult::Failure(err) => { // TODO(#175): For now, ignore cases where the Lean code returned an error due to // an unknown extension function. if !err.contains("jsonToExtFun: unknown extension function") { @@ -200,12 +209,12 @@ pub fn run_val_test( ); } } - Ok(definitional_res) => { + TestResult::Success(definitional_res) => { // Even if the Rust validator succeeds, the definitional validator may // return "impossiblePolicy" due to greater precision. In this case, the // input policy is well-typed, although it is guaranteed to always evaluate // to false. - if definitional_res.validation_errors == vec!["impossiblePolicy".to_string()] { + if definitional_res.errors == vec!["impossiblePolicy".to_string()] { return; } From 0231ad95a7ead1fa3a2cae8f1deaaa2d766757a9 Mon Sep 17 00:00:00 2001 From: Kesha Hietala Date: Mon, 12 Feb 2024 15:18:37 +0000 Subject: [PATCH 03/11] more fixes --- cedar-drt/fuzz/fuzz_targets/formatter.rs | 2 +- .../fuzz_targets/rbac_authorizer_shared.rs | 1 - cedar-drt/fuzz/src/lib.rs | 42 ++++++++++--------- cedar-drt/src/logger.rs | 10 ----- 4 files changed, 24 insertions(+), 31 deletions(-) diff --git a/cedar-drt/fuzz/fuzz_targets/formatter.rs b/cedar-drt/fuzz/fuzz_targets/formatter.rs index 6689b2839..9e3e792ff 100644 --- a/cedar-drt/fuzz/fuzz_targets/formatter.rs +++ b/cedar-drt/fuzz/fuzz_targets/formatter.rs @@ -77,7 +77,7 @@ fn contains_unspecified_entities(p: &StaticPolicy) -> bool { fn attach_comment(p: &str, uuids: &mut Vec) -> String { let mut tokens = lexer::get_token_stream(p).expect("tokens should exist"); for t in tokens.iter_mut() { - let mut ids: Vec = vec![Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4()] + let mut ids: Vec = [Uuid::new_v4(), Uuid::new_v4(), Uuid::new_v4()] .iter() .map(|u| u.to_string()) .collect(); diff --git a/cedar-drt/fuzz/fuzz_targets/rbac_authorizer_shared.rs b/cedar-drt/fuzz/fuzz_targets/rbac_authorizer_shared.rs index 324f14117..5c3ad6d8d 100644 --- a/cedar-drt/fuzz/fuzz_targets/rbac_authorizer_shared.rs +++ b/cedar-drt/fuzz/fuzz_targets/rbac_authorizer_shared.rs @@ -14,7 +14,6 @@ * limitations under the License. */ -use cedar_drt::*; use cedar_drt_inner::*; use cedar_policy_core::ast; use cedar_policy_core::entities::Entities; diff --git a/cedar-drt/fuzz/src/lib.rs b/cedar-drt/fuzz/src/lib.rs index bf794ec30..7f87cb579 100644 --- a/cedar-drt/fuzz/src/lib.rs +++ b/cedar-drt/fuzz/src/lib.rs @@ -20,7 +20,7 @@ mod prt; pub use dump::*; pub use prt::*; -use cedar_policy::cedar_test_impl::{ +pub use cedar_policy::cedar_test_impl::{ time_function, CedarTestImplementation, ErrorComparisonMode, TestResult, }; use cedar_policy::frontend::is_authorized::InterfaceResponse; @@ -75,21 +75,25 @@ pub fn run_eval_test( expected.clone(), ); - // TODO(#175): For now, ignore cases where the definitional code returned an error due to - // an unknown extension function. - if let TestResult::Failure(err) = definitional_res { - if err.contains("jsonToExtFun: unknown extension function") { - return; + match definitional_res { + TestResult::Failure(err) => { + // TODO(#175): Ignore cases where the definitional code returned an error due to + // an unknown extension function. + if err.contains("jsonToExtFun: unknown extension function") { + return; + } + // No other errors are expected + panic!("Unexpected error for {request}\nError: {err}"); + } + TestResult::Success(response) => { + // The definitional interpreter response should be `true` + assert!( + response, + "Incorrect evaluation result for {request}\nExpression:\n{expr}\nEntities:\n{entities}\nExpected value:\n{:?}\n", + expected + ) } } - - // Otherwise, `definitional_res` should be `Ok(true)` - assert_eq!( - definitional_res, - TestResult::Success(true), - "Incorrect evaluation result for {request}\nExpression:\n{expr}\nEntities:\n{entities}\nExpected value:\n{:?}\n", - expected - ) } /// Compare the behavior of the authorizer in `cedar-policy` against a custom Cedar @@ -127,9 +131,9 @@ pub fn run_auth_test( rust_res } else { panic!( - "Unexpected parse error for {request}\nPolicies:\n{}\nEntities:\n{}\nError: {err}", - &policies, &entities - ); + "Unexpected error for {request}\nPolicies:\n{}\nEntities:\n{}\nError: {err}", + &policies, &entities + ); } } TestResult::Success(definitional_res) => { @@ -165,7 +169,7 @@ pub fn run_auth_test( ) }; assert_eq!( - rust_res_for_comparison, definitional_res, + rust_res_for_comparison, definitional_res.response, "Mismatch for {request}\nPolicies:\n{}\nEntities:\n{}", &policies, &entities ); @@ -204,7 +208,7 @@ pub fn run_val_test( // an unknown extension function. if !err.contains("jsonToExtFun: unknown extension function") { panic!( - "Unexpected parse error\nPolicies:\n{}\nSchema:\n{:?}\nError: {err}", + "Unexpected error\nPolicies:\n{}\nSchema:\n{:?}\nError: {err}", &policies, schema ); } diff --git a/cedar-drt/src/logger.rs b/cedar-drt/src/logger.rs index 19c15daa6..ab827f094 100644 --- a/cedar-drt/src/logger.rs +++ b/cedar-drt/src/logger.rs @@ -14,7 +14,6 @@ * limitations under the License. */ use log::warn; -use std::time::{Duration, Instant}; pub const TOTAL_MSG: &str = "total (ns) : "; @@ -32,12 +31,3 @@ pub fn initialize_log() { } }; } - -pub fn time_function(f: F) -> (X, Duration) -where - F: FnOnce() -> X, -{ - let start = Instant::now(); - let result = f(); - (result, start.elapsed()) -} From 04326c6c751fb2f36dcf8ea782ae492404a1fd1a Mon Sep 17 00:00:00 2001 From: Kesha Hietala Date: Fri, 16 Feb 2024 17:50:52 +0000 Subject: [PATCH 04/11] reset submodule --- cedar | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cedar b/cedar index 6838bee0f..b21f84e57 160000 --- a/cedar +++ b/cedar @@ -1 +1 @@ -Subproject commit 6838bee0f87567052aea0b201bfb87d61566839e +Subproject commit b21f84e57d7bd6d0f35d945fb78febb0f76ee22a From 6a8ed74155d93bcfccb97aef04787bd21e2f89a4 Mon Sep 17 00:00:00 2001 From: Kesha Hietala Date: Fri, 16 Feb 2024 18:24:48 +0000 Subject: [PATCH 05/11] add back CedarTestImplementation --- cedar-drt/fuzz/src/lib.rs | 2 +- cedar-drt/src/cedar_test_impl.rs | 230 +++++++++++++++++++++++++++ cedar-drt/src/dafny_java_impl.rs | 31 +++- cedar-drt/src/lean_impl.rs | 33 +++- cedar-drt/src/lib.rs | 1 + cedar-drt/tests/integration_tests.rs | 7 +- 6 files changed, 297 insertions(+), 7 deletions(-) create mode 100644 cedar-drt/src/cedar_test_impl.rs diff --git a/cedar-drt/fuzz/src/lib.rs b/cedar-drt/fuzz/src/lib.rs index 7f87cb579..c7e10e4f5 100644 --- a/cedar-drt/fuzz/src/lib.rs +++ b/cedar-drt/fuzz/src/lib.rs @@ -20,7 +20,7 @@ mod prt; pub use dump::*; pub use prt::*; -pub use cedar_policy::cedar_test_impl::{ +pub use cedar_drt::cedar_test_impl::{ time_function, CedarTestImplementation, ErrorComparisonMode, TestResult, }; use cedar_policy::frontend::is_authorized::InterfaceResponse; diff --git a/cedar-drt/src/cedar_test_impl.rs b/cedar-drt/src/cedar_test_impl.rs new file mode 100644 index 000000000..0a3141869 --- /dev/null +++ b/cedar-drt/src/cedar_test_impl.rs @@ -0,0 +1,230 @@ +/* + * Copyright 2022-2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +//! Definition of a `CedarTestImplementation` trait that describes an +//! implementation of Cedar to use during testing. + +pub use cedar_policy::frontend::is_authorized::InterfaceResponse; +use cedar_policy_core::ast::{Expr, PolicySet, Request, Value}; +use cedar_policy_core::authorizer::Authorizer; +use cedar_policy_core::entities::Entities; +use cedar_policy_core::evaluator::Evaluator; +use cedar_policy_core::extensions::Extensions; +use cedar_policy_validator::{ValidationMode, Validator, ValidatorSchema}; +use serde::Deserialize; +use std::collections::HashMap; +use std::time::{Duration, Instant}; + +/// Return type for `CedarTestImplementation` methods +#[derive(Debug, Deserialize)] +pub enum TestResult { + /// The request succeeded + Success(T), + /// The request failed (e.g., due to a parse error) + Failure(String), +} + +impl TestResult { + /// Get the underlying value of a `TestResult`. + /// # Panics + /// If the `TestResult` is a `Failure`. + /// PANIC SAFETY only used in testing code + #[allow(clippy::panic)] + pub fn expect(self, msg: &str) -> T { + match self { + Self::Success(t) => t, + Self::Failure(err) => panic!("{msg}: {err}"), + } + } +} + +/// Version of `Response` used for testing. Includes an `InterfaceResponse` and +/// a map with timing information. +#[derive(Debug, Deserialize)] +pub struct TestResponse { + /// Actual response + pub response: InterfaceResponse, + /// Timing info in microseconds. This field is a `HashMap` to allow timing + /// multiple components (or none at all). + pub timing_info: HashMap, +} + +/// Version of `ValidationResult` used for testing. +#[derive(Debug, Deserialize)] +pub struct TestValidationResult { + /// Validation errors + pub errors: Vec, + /// Timing info in microseconds. This field is a `HashMap` to allow timing + /// multiple components (or none at all). + pub timing_info: HashMap, +} + +impl TestValidationResult { + /// Check if validation succeeded + pub fn validation_passed(&self) -> bool { + self.errors.is_empty() + } +} + +/// Custom implementation of the Cedar authorizer, evaluator, and validator for testing. +pub trait CedarTestImplementation { + /// Custom authorizer entry point. + fn is_authorized( + &self, + request: &Request, + policies: &PolicySet, + entities: &Entities, + ) -> TestResult; + + /// Custom evaluator entry point. The bool return value indicates the whether + /// evaluating the provided expression produces the expected value. + /// `expected` is optional to allow for the case where no return value is + /// expected due to errors. + fn interpret( + &self, + request: &Request, + entities: &Entities, + expr: &Expr, + enable_extensions: bool, + expected: Option, + ) -> TestResult; + + /// Custom validator entry point. + fn validate( + &self, + schema: &ValidatorSchema, + policies: &PolicySet, + mode: ValidationMode, + ) -> TestResult; + + /// `ErrorComparisonMode` that should be used for this `CedarTestImplementation` + fn error_comparison_mode(&self) -> ErrorComparisonMode; +} + +/// Specifies how errors coming from a `CedarTestImplementation` should be +/// compared against errors coming from the Rust implementation. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum ErrorComparisonMode { + /// Don't compare errors at all; the `CedarTestImplementation` is not + /// expected to produce errors matching the Rust implementation's errors in + /// any way. + /// In fact, the `CedarTestImplementation` will be expected to never report + /// errors. + Ignore, + /// The `CedarTestImplementation` is expected to produce "error messages" that + /// are actually just the id of the erroring policy. This will be compared to + /// ensure that the `CedarTestImplementation` agrees with the Rust + /// implementation on which policies produce errors. + PolicyIds, + /// The `CedarTestImplementation` is expected to produce error messages that + /// exactly match the Rust implementation's error messages' `Display` text. + Full, +} + +/// Basic struct to support implementing the `CedarTestImplementation` trait +#[derive(Debug)] +pub struct RustEngine {} + +impl RustEngine { + /// Create a new `RustEngine` + pub fn new() -> Self { + Self {} + } +} + +/// Timing function +pub fn time_function(f: F) -> (X, Duration) +where + F: FnOnce() -> X, +{ + let start = Instant::now(); + let result = f(); + (result, start.elapsed()) +} + +/// An implementation of `CedarTestImplementation` using `cedar-policy`. +/// Used for running integration tests. +impl CedarTestImplementation for RustEngine { + fn is_authorized( + &self, + request: &Request, + policies: &PolicySet, + entities: &Entities, + ) -> TestResult { + let authorizer = Authorizer::new(); + let (response, duration) = + time_function(|| authorizer.is_authorized(request.clone(), policies, entities)); + // Error messages should only include the policy id to use the + // `ErrorComparisonMode::PolicyIds` mode. + let response = cedar_policy::Response::from(response); + let response = InterfaceResponse::new( + response.decision(), + response.diagnostics().reason().cloned().collect(), + response + .diagnostics() + .errors() + .map(cedar_policy::AuthorizationError::id) + .map(ToString::to_string) + .collect(), + ); + let response = TestResponse { + response, + timing_info: HashMap::from([("authorize".into(), duration.as_micros())]), + }; + TestResult::Success(response) + } + + fn interpret( + &self, + request: &Request, + entities: &Entities, + expr: &Expr, + enable_extensions: bool, + expected: Option, + ) -> TestResult { + let exts = if enable_extensions { + Extensions::all_available() + } else { + Extensions::none() + }; + let evaluator = Evaluator::new(request.clone(), entities, &exts); + let result = evaluator.interpret(expr, &HashMap::default()); + let response = result.ok() == expected; + TestResult::Success(response) + } + + fn validate( + &self, + schema: &ValidatorSchema, + policies: &PolicySet, + mode: ValidationMode, + ) -> TestResult { + let validator = Validator::new(schema.clone()); + let (result, duration) = time_function(|| validator.validate(policies, mode)); + let response = TestValidationResult { + errors: result + .validation_errors() + .map(|err| format!("{err:?}")) + .collect(), + timing_info: HashMap::from([("validate".into(), duration.as_micros())]), + }; + TestResult::Success(response) + } + + fn error_comparison_mode(&self) -> ErrorComparisonMode { + ErrorComparisonMode::PolicyIds + } +} diff --git a/cedar-drt/src/dafny_java_impl.rs b/cedar-drt/src/dafny_java_impl.rs index 6907eaffc..c956b82c0 100644 --- a/cedar-drt/src/dafny_java_impl.rs +++ b/cedar-drt/src/dafny_java_impl.rs @@ -17,9 +17,10 @@ //! Implementation of the [`CedarTestImplementation`] trait for the Cedar Java //! implementation extracted from the Dafny specification. +use crate::cedar_test_impl::*; use crate::definitional_request_types::*; -use cedar_policy::cedar_test_impl::*; use cedar_policy::frontend::is_authorized::InterfaceResponse; +use cedar_policy::integration_testing::{CustomCedarImpl, IntegrationTestValidationResult}; use cedar_policy_core::ast::{Expr, Value}; pub use cedar_policy_core::*; pub use cedar_policy_validator::{ValidationMode, ValidatorSchema}; @@ -401,3 +402,31 @@ impl<'j> CedarTestImplementation for JavaDefinitionalEngine<'j> { ErrorComparisonMode::Ignore } } + +/// Implementation of the trait used for integration testing. +impl<'j> CustomCedarImpl for JavaDefinitionalEngine<'j> { + fn is_authorized( + &self, + request: &ast::Request, + policies: &ast::PolicySet, + entities: &Entities, + ) -> InterfaceResponse { + self.is_authorized(request, policies, entities).response + } + + fn validate( + &self, + schema: cedar_policy_validator::ValidatorSchema, + policies: &ast::PolicySet, + ) -> cedar_policy::integration_testing::IntegrationTestValidationResult { + let response = self.validate( + &schema, + policies, + cedar_policy_validator::ValidationMode::default(), + ); + IntegrationTestValidationResult { + validation_passed: response.validation_passed(), + validation_errors_debug: format!("{:?}", response.errors), + } + } +} diff --git a/cedar-drt/src/lean_impl.rs b/cedar-drt/src/lean_impl.rs index c7f25ddc4..1159c4d25 100644 --- a/cedar-drt/src/lean_impl.rs +++ b/cedar-drt/src/lean_impl.rs @@ -24,8 +24,9 @@ use core::panic; use std::collections::HashMap; use std::{env, ffi::CString}; +use crate::cedar_test_impl::*; use crate::definitional_request_types::*; -use cedar_policy::cedar_test_impl::*; +use cedar_policy::integration_testing::{CustomCedarImpl, IntegrationTestValidationResult}; use cedar_policy_core::ast::{Expr, Value}; pub use cedar_policy_core::*; pub use cedar_policy_validator::{ValidationMode, ValidatorSchema}; @@ -339,3 +340,33 @@ impl CedarTestImplementation for LeanDefinitionalEngine { ErrorComparisonMode::PolicyIds } } + +/// Implementation of the trait used for integration testing. The integration +/// tests expect the calls to `is_authorized` and `validate` to succeed. +impl CustomCedarImpl for LeanDefinitionalEngine { + fn is_authorized( + &self, + request: &ast::Request, + policies: &ast::PolicySet, + entities: &Entities, + ) -> InterfaceResponse { + let response = self + .is_authorized(request, policies, entities) + .expect("Unexpected error from the Lean implementation of `is_authorized`"); + response.response + } + + fn validate( + &self, + schema: cedar_policy_validator::ValidatorSchema, + policies: &ast::PolicySet, + ) -> IntegrationTestValidationResult { + let response = self + .validate(&schema, policies) + .expect("Unexpected error from the Lean implementation of `validate`"); + IntegrationTestValidationResult { + validation_passed: response.validation_passed(), + validation_errors_debug: format!("{:?}", response.errors), + } + } +} diff --git a/cedar-drt/src/lib.rs b/cedar-drt/src/lib.rs index 088ea3e52..b1879ed5c 100644 --- a/cedar-drt/src/lib.rs +++ b/cedar-drt/src/lib.rs @@ -14,6 +14,7 @@ * limitations under the License. */ +pub mod cedar_test_impl; mod dafny_java_impl; mod definitional_request_types; mod lean_impl; diff --git a/cedar-drt/tests/integration_tests.rs b/cedar-drt/tests/integration_tests.rs index d0430f20f..06d754f8d 100644 --- a/cedar-drt/tests/integration_tests.rs +++ b/cedar-drt/tests/integration_tests.rs @@ -17,9 +17,8 @@ //! Integration test that runs the handwritten test cases from //! `cedar-integration-tests` on the definitional implementation. -use cedar_policy::cedar_test_impl::CedarTestImplementation; use cedar_policy::integration_testing::{ - perform_integration_test_from_json_custom, resolve_integration_test_path, + perform_integration_test_from_json_custom, resolve_integration_test_path, CustomCedarImpl, }; use cedar_drt::*; @@ -31,7 +30,7 @@ fn folder() -> &'static Path { Path::new("tests") } -fn run_integration_tests(test_impl: &impl CedarTestImplementation) { +fn run_integration_tests(custom_impl: &dyn CustomCedarImpl) { let integration_tests_folder = resolve_integration_test_path(folder()); // find all `*.json` files in the integration tests folder let test_jsons = WalkDir::new(&integration_tests_folder) @@ -48,7 +47,7 @@ fn run_integration_tests(test_impl: &impl CedarTestImplementation) { // `#[test]` fails, the user can know which test case failed by looking // for the last "Running integration test" message before the failure. println!("Running integration test: {:?}", test_json); - perform_integration_test_from_json_custom(&test_json, test_impl); + perform_integration_test_from_json_custom(&test_json, Some(custom_impl)); println!("Integration test succeeded: {:?}", test_json); } } From e8d2bd2115ec912b6d42294f0a2933d1b4869215 Mon Sep 17 00:00:00 2001 From: Kesha Hietala Date: Fri, 16 Feb 2024 20:03:56 +0000 Subject: [PATCH 06/11] update submodule --- cedar | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cedar b/cedar index b21f84e57..2af5282fe 160000 --- a/cedar +++ b/cedar @@ -1 +1 @@ -Subproject commit b21f84e57d7bd6d0f35d945fb78febb0f76ee22a +Subproject commit 2af5282fe692f60af4c7f90304ec9ff40d25659e From 107d742a1fe49dea86095cf9c9a0685f0dd0c1ad Mon Sep 17 00:00:00 2001 From: Kesha Hietala Date: Sat, 17 Feb 2024 00:21:04 +0000 Subject: [PATCH 07/11] first pass at benchmark file --- cedar | 2 +- cedar-drt/Cargo.toml | 1 + cedar-drt/tests/benchmark.rs | 187 +++++++++++++++++++++++++++ cedar-drt/tests/integration_tests.rs | 16 ++- 4 files changed, 199 insertions(+), 7 deletions(-) create mode 100644 cedar-drt/tests/benchmark.rs diff --git a/cedar b/cedar index 2af5282fe..aef194028 160000 --- a/cedar +++ b/cedar @@ -1 +1 @@ -Subproject commit 2af5282fe692f60af4c7f90304ec9ff40d25659e +Subproject commit aef194028ff323277c2e2d2541e87b181badfbb7 diff --git a/cedar-drt/Cargo.toml b/cedar-drt/Cargo.toml index 547959b25..548994b9f 100644 --- a/cedar-drt/Cargo.toml +++ b/cedar-drt/Cargo.toml @@ -21,6 +21,7 @@ smol_str = { version = "0.2", features = ["serde"] } [dev-dependencies] walkdir = "2.4" +statrs = "0.16" [dependencies.uuid] version = "1.3.1" diff --git a/cedar-drt/tests/benchmark.rs b/cedar-drt/tests/benchmark.rs new file mode 100644 index 000000000..252781fcb --- /dev/null +++ b/cedar-drt/tests/benchmark.rs @@ -0,0 +1,187 @@ +/* + * Copyright 2022-2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +//! Run the standard and definitional implementations of Cedar on the integration +//! tests are record performance results. + +mod integration_tests; + +use cedar_drt::ast::{PolicySet, Request}; +use cedar_drt::cedar_test_impl::*; +use cedar_drt::{ValidatorSchema, ValidationMode}; +use cedar_drt::{Entities, JavaDefinitionalEngine, LeanDefinitionalEngine}; +use cedar_policy::integration_testing::*; +use integration_tests::get_tests; +use statrs::statistics::{Data, OrderStatistics}; +use std::{collections::HashMap, path::Path}; + +const NUM_TRIALS: u32 = 10; + +/// Parse a file in the integration test format, ignoring the expected +/// authorization/validation results. +fn parse_test(jsonfile: impl AsRef) -> (PolicySet, Entities, ValidatorSchema, Vec) { + let jsonfile = resolve_integration_test_path(jsonfile); + let test_name: String = jsonfile.display().to_string(); + let jsonstr = std::fs::read_to_string(jsonfile.as_path()) + .unwrap_or_else(|e| panic!("error reading from file {test_name}: {e}")); + let test: JsonTest = + serde_json::from_str(&jsonstr).unwrap_or_else(|e| panic!("error parsing {test_name}: {e}")); + let policies = parse_policies_from_test_internal(&test); + let schema = parse_schema_from_test(&test); + let schema_internal = parse_schema_from_test_internal(&test); + let entities = parse_entities_from_test_internal(&test, &schema); + let requests = test + .requests + .into_iter() + .map(|json_request| parse_request_from_test_internal(&json_request, &schema, &test_name)) + .collect(); + (policies, entities, schema_internal, requests) +} + +fn median(data: Vec) -> f64 { + let mut data = Data::new(data); + data.median() +} + +fn p90(data: Vec) -> f64 { + let mut data = Data::new(data); + data.percentile(90) +} + +fn p99(data: Vec) -> f64 { + let mut data = Data::new(data); + data.percentile(99) +} + +/// Run every input in `folder()` through the provided Cedar test implementation, +/// recording the time(s) required for authorization over NUM_TRIALS trials. The +/// output reports the median trial value for each input. +/// +/// There are two statistics measured: "total" for end-to-end authorization time +/// and (optionally) "authorize" for the reported _core_ authorization time, which +/// ignores time required to interface with the test implementation. +fn get_authorization_timing_results( + custom_impl: &dyn CedarTestImplementation, +) -> HashMap<&str, Vec> { + let tests = get_tests(); + let mut results = HashMap::new(); + results.insert("total", Vec::new()); + results.insert("authorize", Vec::new()); + for test in tests { + let test_name = String::from(test.file_name().unwrap().to_str().unwrap()); + println!("Running test: {:?}", test_name); + let (policies, entities, _schema, requests) = parse_test(test); + for request in requests { + let mut total_results = Vec::new(); + let mut auth_results = Vec::new(); + for _i in 0..NUM_TRIALS { + let (response, duration) = + time_function(|| custom_impl.is_authorized(&request, &policies, &entities)); + let response = response.expect("Unexpected test failure"); + total_results.push(duration.as_micros() as f64); + if response.timing_info.contains_key("authorize") { + auth_results.push(response.timing_info["authorize"] as f64); + } + } + results + .get_mut("total") + .unwrap() + .push(median(total_results)); + results + .get_mut("authorize") + .unwrap() + .push(median(auth_results)); + } + } + results +} + +/// Run every input in `folder()` through the provided Cedar test implementation, +/// recording the time(s) required for validation over NUM_TRIALS trials. The +/// output reports the median trial value for each input. +/// +/// There are two statistics measured: "total" for end-to-end validation time +/// and (optionally) "validate" for the reported _core_ validation time, which +/// ignores time required to interface with the test implementation. +fn get_validation_timing_results( + custom_impl: &dyn CedarTestImplementation, +) -> HashMap<&str, Vec> { + let tests = get_tests(); + let mut results = HashMap::new(); + results.insert("total", Vec::new()); + results.insert("validate", Vec::new()); + for test in tests { + let test_name = String::from(test.file_name().unwrap().to_str().unwrap()); + println!("Running test: {:?}", test_name); + let (policies, _entities, schema, _requests) = parse_test(test); + let mut total_results = Vec::new(); + let mut val_results = Vec::new(); + for _i in 0..NUM_TRIALS { + let (response, duration) = + time_function(|| custom_impl.validate(&schema, &policies, ValidationMode::Strict)); + let response = response.expect("Unexpected test failure"); + total_results.push(duration.as_micros() as f64); + if response.timing_info.contains_key("validate") { + val_results.push(response.timing_info["validate"] as f64); + } + } + results + .get_mut("total") + .unwrap() + .push(median(total_results)); + results + .get_mut("validate") + .unwrap() + .push(median(val_results)); + } + results +} + +/// Print out a summary of the results +fn print_summary(auth_times: HashMap<&str, Vec>, val_times: HashMap<&str, Vec>) { + for (key, value) in auth_times.into_iter() { + println!("Authorization - {key}"); + println!("\tMedian: {:.1} micros", median(value.clone())); + println!("\tp90: {:.1} micros", p90(value.clone())); + println!("\tp99: {:.1} micros", p99(value)); + } + for (key, value) in val_times.into_iter() { + println!("Validation - {key}"); + println!("\tMedian: {:.1} micros", median(value.clone())); + println!("\tp90: {:.1} micros", p90(value.clone())); + println!("\tp99: {:.1} micros", p99(value)); + } +} + +#[test] +fn run_all_tests() { + let rust_impl = RustEngine::new(); + let lean_def_impl = LeanDefinitionalEngine::new(); + let java_def_impl = + JavaDefinitionalEngine::new().expect("failed to create Dafny definitional engine"); + + let auth_times = get_authorization_timing_results(&rust_impl); + let val_times = get_validation_timing_results(&rust_impl); + print_summary(auth_times, val_times); + + let auth_times = get_authorization_timing_results(&rust_impl); + let val_times = get_validation_timing_results(&rust_impl); + print_summary(auth_times, val_times); + + let auth_times = get_authorization_timing_results(&rust_impl); + let val_times = get_validation_timing_results(&rust_impl); + print_summary(auth_times, val_times); +} diff --git a/cedar-drt/tests/integration_tests.rs b/cedar-drt/tests/integration_tests.rs index 06d754f8d..72738d22a 100644 --- a/cedar-drt/tests/integration_tests.rs +++ b/cedar-drt/tests/integration_tests.rs @@ -22,7 +22,7 @@ use cedar_policy::integration_testing::{ }; use cedar_drt::*; -use std::path::Path; +use std::path::{Path, PathBuf}; use walkdir::WalkDir; /// Path of the folder containing the integration tests @@ -30,10 +30,11 @@ fn folder() -> &'static Path { Path::new("tests") } -fn run_integration_tests(custom_impl: &dyn CustomCedarImpl) { +/// Pull out the relevant tests in `folder()` +pub fn get_tests() -> impl Iterator { let integration_tests_folder = resolve_integration_test_path(folder()); // find all `*.json` files in the integration tests folder - let test_jsons = WalkDir::new(&integration_tests_folder) + WalkDir::new(&integration_tests_folder) .into_iter() .map(|e| e.expect("failed to access file in tests").into_path()) // ignore non-JSON files (and directories, which don't have an extension) @@ -41,8 +42,11 @@ fn run_integration_tests(custom_impl: &dyn CustomCedarImpl) { p.extension() .map(|ext| ext.eq_ignore_ascii_case("json")) .unwrap_or(false) - }); - for test_json in test_jsons { + }) +} + +fn run_integration_tests(custom_impl: &dyn CustomCedarImpl) { + for test_json in get_tests() { // These messages are for progress reporting and so that if the // `#[test]` fails, the user can know which test case failed by looking // for the last "Running integration test" message before the failure. @@ -52,7 +56,7 @@ fn run_integration_tests(custom_impl: &dyn CustomCedarImpl) { } } -#[test] +// #[test] fn integration_tests_on_def_impl() { //WARNING: We need to create lean def engine first so the JVM signal handlers are aware of it. //If this needs to change at some point in the future, you'll need to add libjsig.so to LD_PRELOAD From 0188c44f165e17f6b273825d2a16879c15bb871f Mon Sep 17 00:00:00 2001 From: Kesha Hietala Date: Tue, 20 Feb 2024 15:10:15 +0000 Subject: [PATCH 08/11] cleanup --- cedar | 2 +- cedar-drt/src/lean_impl.rs | 2 +- cedar-drt/tests/benchmark.rs | 16 ++++++---------- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/cedar b/cedar index aef194028..f14442eb4 160000 --- a/cedar +++ b/cedar @@ -1 +1 @@ -Subproject commit aef194028ff323277c2e2d2541e87b181badfbb7 +Subproject commit f14442eb4fda77b7f73d4517251265a315754e11 diff --git a/cedar-drt/src/lean_impl.rs b/cedar-drt/src/lean_impl.rs index 1159c4d25..963bde018 100644 --- a/cedar-drt/src/lean_impl.rs +++ b/cedar-drt/src/lean_impl.rs @@ -277,7 +277,7 @@ impl LeanDefinitionalEngine { }; let response = TestValidationResult { errors: validation_errors, - timing_info: HashMap::from([("validation".into(), resp.duration / 1000)]), + timing_info: HashMap::from([("validate".into(), resp.duration / 1000)]), }; TestResult::Success(response) } diff --git a/cedar-drt/tests/benchmark.rs b/cedar-drt/tests/benchmark.rs index 252781fcb..75f43e9b2 100644 --- a/cedar-drt/tests/benchmark.rs +++ b/cedar-drt/tests/benchmark.rs @@ -22,7 +22,7 @@ mod integration_tests; use cedar_drt::ast::{PolicySet, Request}; use cedar_drt::cedar_test_impl::*; use cedar_drt::{ValidatorSchema, ValidationMode}; -use cedar_drt::{Entities, JavaDefinitionalEngine, LeanDefinitionalEngine}; +use cedar_drt::{Entities, LeanDefinitionalEngine}; use cedar_policy::integration_testing::*; use integration_tests::get_tests; use statrs::statistics::{Data, OrderStatistics}; @@ -169,19 +169,15 @@ fn print_summary(auth_times: HashMap<&str, Vec>, val_times: HashMap<&str, V #[test] fn run_all_tests() { let rust_impl = RustEngine::new(); - let lean_def_impl = LeanDefinitionalEngine::new(); - let java_def_impl = - JavaDefinitionalEngine::new().expect("failed to create Dafny definitional engine"); + let lean_impl = LeanDefinitionalEngine::new(); + println!("Running Rust implementation..."); let auth_times = get_authorization_timing_results(&rust_impl); let val_times = get_validation_timing_results(&rust_impl); print_summary(auth_times, val_times); - let auth_times = get_authorization_timing_results(&rust_impl); - let val_times = get_validation_timing_results(&rust_impl); - print_summary(auth_times, val_times); - - let auth_times = get_authorization_timing_results(&rust_impl); - let val_times = get_validation_timing_results(&rust_impl); + println!("Running Lean implementation..."); + let auth_times = get_authorization_timing_results(&lean_impl); + let val_times = get_validation_timing_results(&lean_impl); print_summary(auth_times, val_times); } From c9770b2abebb1cc24837bdd7b27966b5643dffce Mon Sep 17 00:00:00 2001 From: Kesha Hietala Date: Wed, 21 Feb 2024 23:11:47 +0000 Subject: [PATCH 09/11] cleanup --- cedar-drt/tests/benchmark.rs | 19 +++++----- cedar-drt/tests/integration_tests.rs | 57 +++++++++++++++++++++++----- 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/cedar-drt/tests/benchmark.rs b/cedar-drt/tests/benchmark.rs index 75f43e9b2..74edb4a47 100644 --- a/cedar-drt/tests/benchmark.rs +++ b/cedar-drt/tests/benchmark.rs @@ -21,10 +21,10 @@ mod integration_tests; use cedar_drt::ast::{PolicySet, Request}; use cedar_drt::cedar_test_impl::*; -use cedar_drt::{ValidatorSchema, ValidationMode}; use cedar_drt::{Entities, LeanDefinitionalEngine}; +use cedar_drt::{ValidationMode, ValidatorSchema}; use cedar_policy::integration_testing::*; -use integration_tests::get_tests; +use integration_tests::get_corpus_tests; use statrs::statistics::{Data, OrderStatistics}; use std::{collections::HashMap, path::Path}; @@ -66,7 +66,7 @@ fn p99(data: Vec) -> f64 { data.percentile(99) } -/// Run every input in `folder()` through the provided Cedar test implementation, +/// Run every input in the corpus tests through the provided Cedar test implementation, /// recording the time(s) required for authorization over NUM_TRIALS trials. The /// output reports the median trial value for each input. /// @@ -76,13 +76,11 @@ fn p99(data: Vec) -> f64 { fn get_authorization_timing_results( custom_impl: &dyn CedarTestImplementation, ) -> HashMap<&str, Vec> { - let tests = get_tests(); + let tests = get_corpus_tests(); let mut results = HashMap::new(); results.insert("total", Vec::new()); results.insert("authorize", Vec::new()); for test in tests { - let test_name = String::from(test.file_name().unwrap().to_str().unwrap()); - println!("Running test: {:?}", test_name); let (policies, entities, _schema, requests) = parse_test(test); for request in requests { let mut total_results = Vec::new(); @@ -109,7 +107,7 @@ fn get_authorization_timing_results( results } -/// Run every input in `folder()` through the provided Cedar test implementation, +/// Run every input in the corpus tests through the provided Cedar test implementation, /// recording the time(s) required for validation over NUM_TRIALS trials. The /// output reports the median trial value for each input. /// @@ -119,13 +117,11 @@ fn get_authorization_timing_results( fn get_validation_timing_results( custom_impl: &dyn CedarTestImplementation, ) -> HashMap<&str, Vec> { - let tests = get_tests(); + let tests = get_corpus_tests(); let mut results = HashMap::new(); results.insert("total", Vec::new()); results.insert("validate", Vec::new()); for test in tests { - let test_name = String::from(test.file_name().unwrap().to_str().unwrap()); - println!("Running test: {:?}", test_name); let (policies, _entities, schema, _requests) = parse_test(test); let mut total_results = Vec::new(); let mut val_results = Vec::new(); @@ -167,6 +163,9 @@ fn print_summary(auth_times: HashMap<&str, Vec>, val_times: HashMap<&str, V } #[test] +// Currently, running this in conjunction with existing tests will cause an error (#227). +// In order see the printed output from this test, run `cargo test -- --ignored --nocapture`. +// #[ignore] fn run_all_tests() { let rust_impl = RustEngine::new(); let lean_impl = LeanDefinitionalEngine::new(); diff --git a/cedar-drt/tests/integration_tests.rs b/cedar-drt/tests/integration_tests.rs index d7680975b..bc72e79d9 100644 --- a/cedar-drt/tests/integration_tests.rs +++ b/cedar-drt/tests/integration_tests.rs @@ -25,16 +25,21 @@ use cedar_drt::*; use std::path::{Path, PathBuf}; use walkdir::WalkDir; -/// Path of the folder containing the integration tests -fn folder() -> &'static Path { +/// Path of the folder containing the (handwritten) integration tests +fn integration_test_folder() -> &'static Path { Path::new("tests") } -/// Pull out the relevant tests in `folder()` -pub fn get_tests() -> impl Iterator { - let integration_tests_folder = resolve_integration_test_path(folder()); +/// Path of the folder containing the autogenerated corpus tests +fn corpus_test_folder() -> &'static Path { + Path::new("corpus_tests") +} + +/// Pull out the relevant tests in `integration_test_folder()` +pub fn get_integration_tests() -> impl Iterator { + let tests_folder = resolve_integration_test_path(integration_test_folder()); // find all `*.json` files in the integration tests folder - WalkDir::new(&integration_tests_folder) + WalkDir::new(&tests_folder) .into_iter() .map(|e| e.expect("failed to access file in tests").into_path()) // ignore non-JSON files (and directories, which don't have an extension) @@ -45,8 +50,31 @@ pub fn get_tests() -> impl Iterator { }) } -fn run_integration_tests(custom_impl: &dyn CustomCedarImpl) { - for test_json in get_tests() { +/// Pull out the relevant tests in `corpus_test_folder()` +pub fn get_corpus_tests() -> impl Iterator { + let tests_folder = resolve_integration_test_path(corpus_test_folder()); + // find all `*.json` files, ignoring those that start with `schema_` + WalkDir::new(&tests_folder) + .into_iter() + .map(|e| { + e.expect("failed to access file in corpus_tests") + .into_path() + }) + .filter(|p| { + let filename = p + .file_name() + .expect("didn't expect subdirectories in corpus_tests") + .to_str() + .expect("expected filenames to be valid UTF-8"); + p.extension() + .map(|ext| ext.eq_ignore_ascii_case("json")) + .unwrap_or(false) + && !filename.starts_with("schema_") + }) +} + +fn run_tests(custom_impl: &dyn CustomCedarImpl, tests: impl Iterator) { + for test_json in tests { // These messages are for progress reporting and so that if the // `#[test]` fails, the user can know which test case failed by looking // for the last "Running integration test" message before the failure. @@ -56,8 +84,19 @@ fn run_integration_tests(custom_impl: &dyn CustomCedarImpl) { } } -// #[test] +fn run_integration_tests(custom_impl: &dyn CustomCedarImpl) { + run_tests(custom_impl, get_integration_tests()); +} + +fn run_corpus_tests(custom_impl: &dyn CustomCedarImpl) { + run_tests(custom_impl, get_corpus_tests()); +} + +#[test] fn integration_tests_on_def_impl() { let lean_def_impl = LeanDefinitionalEngine::new(); run_integration_tests(&lean_def_impl); + + // This test may fail due to differences in precision between the Rust & Lean validators (#226) + // run_corpus_tests(&lean_def_impl); } From 7f0facd8857159461ad86eec44218ca8f592e9f3 Mon Sep 17 00:00:00 2001 From: Kesha Hietala Date: Wed, 21 Feb 2024 23:13:18 +0000 Subject: [PATCH 10/11] update submodule --- cedar | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cedar b/cedar index f14442eb4..687d0f5d6 160000 --- a/cedar +++ b/cedar @@ -1 +1 @@ -Subproject commit f14442eb4fda77b7f73d4517251265a315754e11 +Subproject commit 687d0f5d62b80556dfaa2b2c296703c50c332b4f From 8d8a86668f91107842e8c9c7860058252bf57596 Mon Sep 17 00:00:00 2001 From: Kesha Hietala Date: Thu, 22 Feb 2024 14:50:45 +0000 Subject: [PATCH 11/11] updates from review --- cedar-drt/fuzz/src/lib.rs | 4 ++-- cedar-drt/src/cedar_test_impl.rs | 12 ++++++++---- cedar-drt/src/lean_impl.rs | 4 ++-- cedar-drt/tests/benchmark.rs | 6 +++--- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/cedar-drt/fuzz/src/lib.rs b/cedar-drt/fuzz/src/lib.rs index a3a0b8ecf..16328e9da 100644 --- a/cedar-drt/fuzz/src/lib.rs +++ b/cedar-drt/fuzz/src/lib.rs @@ -83,13 +83,13 @@ pub fn run_eval_test( return; } // No other errors are expected - panic!("Unexpected error for {request}\nError: {err}"); + panic!("Unexpected error for {request}\nExpression: {expr}\nError: {err}"); } TestResult::Success(response) => { // The definitional interpreter response should be `true` assert!( response, - "Incorrect evaluation result for {request}\nExpression:\n{expr}\nEntities:\n{entities}\nExpected value:\n{:?}\n", + "Incorrect evaluation result for {request}\nExpression: {expr}\nEntities:\n{entities}\nExpected value:\n{:?}\n", expected ) } diff --git a/cedar-drt/src/cedar_test_impl.rs b/cedar-drt/src/cedar_test_impl.rs index 0a3141869..da7bb4227 100644 --- a/cedar-drt/src/cedar_test_impl.rs +++ b/cedar-drt/src/cedar_test_impl.rs @@ -51,6 +51,10 @@ impl TestResult { } } +/// Simple wrapper around u128 to remind ourselves that timing info is in microseconds. +#[derive(Debug, Deserialize)] +pub struct Micros(pub u128); + /// Version of `Response` used for testing. Includes an `InterfaceResponse` and /// a map with timing information. #[derive(Debug, Deserialize)] @@ -59,7 +63,7 @@ pub struct TestResponse { pub response: InterfaceResponse, /// Timing info in microseconds. This field is a `HashMap` to allow timing /// multiple components (or none at all). - pub timing_info: HashMap, + pub timing_info: HashMap, } /// Version of `ValidationResult` used for testing. @@ -69,7 +73,7 @@ pub struct TestValidationResult { pub errors: Vec, /// Timing info in microseconds. This field is a `HashMap` to allow timing /// multiple components (or none at all). - pub timing_info: HashMap, + pub timing_info: HashMap, } impl TestValidationResult { @@ -182,7 +186,7 @@ impl CedarTestImplementation for RustEngine { ); let response = TestResponse { response, - timing_info: HashMap::from([("authorize".into(), duration.as_micros())]), + timing_info: HashMap::from([("authorize".into(), Micros(duration.as_micros()))]), }; TestResult::Success(response) } @@ -219,7 +223,7 @@ impl CedarTestImplementation for RustEngine { .validation_errors() .map(|err| format!("{err:?}")) .collect(), - timing_info: HashMap::from([("validate".into(), duration.as_micros())]), + timing_info: HashMap::from([("validate".into(), Micros(duration.as_micros()))]), }; TestResult::Success(response) } diff --git a/cedar-drt/src/lean_impl.rs b/cedar-drt/src/lean_impl.rs index 963bde018..73c23f23f 100644 --- a/cedar-drt/src/lean_impl.rs +++ b/cedar-drt/src/lean_impl.rs @@ -182,7 +182,7 @@ impl LeanDefinitionalEngine { .collect(); TestResult::Success(TestResponse { response: InterfaceResponse::new(decision, reason, errors), - timing_info: HashMap::from([("authorize".into(), auth_time)]), + timing_info: HashMap::from([("authorize".into(), Micros(auth_time))]), }) } AuthorizationResponse::Error(err) => TestResult::Failure(err), @@ -277,7 +277,7 @@ impl LeanDefinitionalEngine { }; let response = TestValidationResult { errors: validation_errors, - timing_info: HashMap::from([("validate".into(), resp.duration / 1000)]), + timing_info: HashMap::from([("validate".into(), Micros(resp.duration / 1000))]), }; TestResult::Success(response) } diff --git a/cedar-drt/tests/benchmark.rs b/cedar-drt/tests/benchmark.rs index 74edb4a47..5e24fbf3c 100644 --- a/cedar-drt/tests/benchmark.rs +++ b/cedar-drt/tests/benchmark.rs @@ -91,7 +91,7 @@ fn get_authorization_timing_results( let response = response.expect("Unexpected test failure"); total_results.push(duration.as_micros() as f64); if response.timing_info.contains_key("authorize") { - auth_results.push(response.timing_info["authorize"] as f64); + auth_results.push(response.timing_info["authorize"].0 as f64); } } results @@ -131,7 +131,7 @@ fn get_validation_timing_results( let response = response.expect("Unexpected test failure"); total_results.push(duration.as_micros() as f64); if response.timing_info.contains_key("validate") { - val_results.push(response.timing_info["validate"] as f64); + val_results.push(response.timing_info["validate"].0 as f64); } } results @@ -165,7 +165,7 @@ fn print_summary(auth_times: HashMap<&str, Vec>, val_times: HashMap<&str, V #[test] // Currently, running this in conjunction with existing tests will cause an error (#227). // In order see the printed output from this test, run `cargo test -- --ignored --nocapture`. -// #[ignore] +#[ignore] fn run_all_tests() { let rust_impl = RustEngine::new(); let lean_impl = LeanDefinitionalEngine::new();