-
Notifications
You must be signed in to change notification settings - Fork 6
fix(core): Fix obligation smoke test #687
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @c-r33d, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines an existing end-to-end smoke test to improve its robustness and maintainability. By adjusting the assertion to look for a broader phrase, the test will continue to validate the core functionality of obligation handling without being brittle to specific obligation value changes. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request fixes an obligation smoke test by making the assertions more strict. In the test case for a user not entitled to data, the check is updated to refute the presence of the string 'required obligations' entirely, rather than just a specific obligation FQN. This is a good improvement as it better enforces that no information about obligations is leaked when decryption is denied due to lack of entitlement. The changes in e2e/encrypt-decrypt.bats are correct and enhance the test's robustness.
X-Test Results✅ js-v0.4.34 |
|
Looks good, but won't pass until this is merged first (fixing the API issue): opentdf/platform#2847 Successful test run here against this branch: https://github.com/opentdf/platform/actions/runs/18925941963/job/54032854499?pr=2847 |
|
/backport |
1.) Obligation smoke test should just for the presence of`required obligation` (cherry picked from commit c8e9b20)
|
Successfully created backport PR for |
1.) Obligation smoke test should just for the presence of`required obligation` (cherry picked from commit c8e9b20)
# Description Backport of #687 to `release/v0.26`. Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
1.) Obligation smoke test should just for the presence of
required obligation