Skip to content

adds FFI Tests - #320

Merged
muditchaudhary merged 1 commit into
cedar-policy:mainfrom
swabyra:tests-for-non-refactors
Jul 16, 2025
Merged

adds FFI Tests #320
muditchaudhary merged 1 commit into
cedar-policy:mainfrom
swabyra:tests-for-non-refactors

Conversation

@swabyra

@swabyra swabyra commented Jul 9, 2025

Copy link
Copy Markdown
Contributor

Issue #, if available:
Tests for non refactored methods along with some tests cases I had written for JLIST.RS over the weekend.
Description of changes:

Comment thread CedarJavaFFI/src/interface.rs Outdated
@swabyra
swabyra force-pushed the tests-for-non-refactors branch from 519189a to fbe42ef Compare July 14, 2025 15:03
Comment thread CedarJavaFFI/src/interface.rs
Comment thread CedarJavaFFI/src/interface.rs Outdated
result
);

let jval = result.unwrap();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: variable naming can be improved here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I updated it and made the names more detailed not 100% sure if its how you guys want it tho

Comment thread CedarJavaFFI/src/interface.rs
Comment thread CedarJavaFFI/src/interface.rs Outdated
Comment thread CedarJavaFFI/src/interface.rs
Comment thread CedarJavaFFI/src/interface.rs
Comment thread CedarJavaFFI/src/interface.rs Outdated
Comment thread CedarJavaFFI/src/interface.rs Outdated
@swabyra
swabyra force-pushed the tests-for-non-refactors branch from fbe42ef to b322546 Compare July 15, 2025 03:07
use jni::JavaVM;
use std::sync::LazyLock;

// Static JVM to be used by all the tests. LazyLock for thread-safe lazy initialization

@mark-creamer-amazon mark-creamer-amazon Jul 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I find the comment helpful imo! But fine either way

@muditchaudhary muditchaudhary Jul 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, we can add this comment and remove the following comment from interface.rs

///
///
///  REPO MADE FOR TESTS OF NON-REFACTORED METHODS
///
///

+ need to run cargo fmt as formatting tests are failing.

Rest looks good to me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will do

Comment on lines +1225 to +1233
"User": {
"memberOfTypes": ["Group"]
},
"Group": {},
"File": {}
},
"actions": {
"read": {
"appliesTo": {
"principalTypes": ["User"],
"resourceTypes": ["File"]
}
}
}
}

@mark-creamer-amazon mark-creamer-amazon Jul 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: if there's another commit coming on this PR, could consider fixing the indentation here and elsewhere while you're at it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will be looking over the schemas

Comment thread CedarJavaFFI/src/jlist.rs Outdated
Signed-off-by: Raheim Swaby <swabyra@amazon.com>
@swabyra
swabyra force-pushed the tests-for-non-refactors branch from b322546 to b6cf365 Compare July 15, 2025 15:30

@mark-creamer-amazon mark-creamer-amazon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

@muditchaudhary muditchaudhary changed the title Tests for non refactors adds FFI Tests Jul 16, 2025
@muditchaudhary
muditchaudhary merged commit 748f28b into cedar-policy:main Jul 16, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants