Skip to content
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

Remove legacy plugin tests #34

Merged
merged 4 commits into from
Apr 20, 2024
Merged

Remove legacy plugin tests #34

merged 4 commits into from
Apr 20, 2024

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Apr 6, 2024

For context, see discussion in JuliaLang/julia#53891

@codecov-commenter
Copy link

codecov-commenter commented Apr 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.74%. Comparing base (ffca279) to head (9734554).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #34      +/-   ##
==========================================
- Coverage   76.80%   70.74%   -6.06%     
==========================================
  Files           2        2              
  Lines        1095     1128      +33     
==========================================
- Hits          841      798      -43     
- Misses        254      330      +76     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mkitti
Copy link
Contributor

mkitti commented Apr 7, 2024

Not all of those ciphers are legacy ones...

# https://www.openssl.org/docs/man3.0/man7/OSSL_PROVIDER-legacy.html
@testset "Encrypt" begin
if OpenSSL.version_number() ≥ v"3"
OpenSSL.load_legacy_provider()

Choose a reason for hiding this comment

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

The load_legacy_provider function and associated code would go away, I suppose.

EvpBlowFishECB(), # legacy
#EvpBlowFishCFB(), // not supported
EvpBlowFishOFB(), # legacy
EvpAES128CBC(),

Choose a reason for hiding this comment

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

EvpAES128CBC, EvpAES128ECB and EvpAES128OFB aren't marked as legacy and should remain.

end

@testset "EncryptCustomKey" begin
# EvpBlowFishECB is legacy, consider using EvpAES128ECB instead

Choose a reason for hiding this comment

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

Instead of being removed, this test could be moved to a non-legacy cipher as suggested in the comment.

@giordano
Copy link

@quinnj gentle bump: could you please have a look at the comments above? 🙂

@quinnj quinnj merged commit 29cd566 into main Apr 20, 2024
12 checks passed
@quinnj quinnj deleted the jq-remove-legacy-tests branch April 20, 2024 05:57
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.

5 participants