-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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
Backport HMACContext to 3.x #48869
Backport HMACContext to 3.x #48869
Conversation
e8062a0
to
c9ef1b1
Compare
07e9bb6
to
9795a27
Compare
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.
The script you mention in the PR is not the one for HMAContext, but for AESContext (which was already backported).
The tests in main/tests/*
cannot be run as they are not called in main_test.cpp
and would not compile anyway due to missing references.
We can either skip those, or add the missing functions.
Thank you for the review @Faless! I've made the changes you requested, since those tests won't run anyway ( I updated the PR description with the ammended HMAC test, sorry my mistake for that. I checked and it's working also. I couldn't find how to run tests locally, is that documented anywhere? |
|
0f88bdc
to
dd62923
Compare
Thank you! 15m ⚑
▶ ./bin/godot.osx.tools.x86_64 --test crypto
arguments
0: ./bin/godot.osx.tools.x86_64
1: --test
2: crypto
Current path: /Users/will/src/godot
Godot Engine v3.4.beta.custom_build.0f88bdc07 - https://godotengine.org
OpenGL ES 3.0 Renderer: Intel(R) Iris(TM) Plus Graphics 655
OpenGL ES Batching: ON
Registered camera FaceTime HD Camera (Built-in) with id 1 position 0 at index 0
PASS
Passed 1 of 1 tests |
Fix headers Fix docs formatting Changes for PR Fix tests
dd62923
to
3f60626
Compare
#include "core/crypto/crypto.h" | ||
#include "core/os/os.h" | ||
|
||
namespace TestCrypto { |
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.
Does the test pass when compiling with module_mbedtls_enabled=no
?
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.
Good question, I compiled just now using:
scons platform=osx arch=x86_64 module_mbedtls_enabled=no --jobs=$(sysctl -n hw.logicalcpu)
And tests are passing, is that correct?
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.
Looks good, thanks! 👍
Thanks! And congrats for your first merged Godot contribution 🎉 |
Thank you @akien-mga @Faless! |
I backported this PR #43536
Thank you @jonbonazza!
I also tested with this simple script from the docs:
godotengine/godot-proposals#1098
godotengine/godot-proposals#1127