Skip to content

Remove legacy plugin tests#34

Merged
quinnj merged 4 commits intomainfrom
jq-remove-legacy-tests
Apr 20, 2024
Merged

Remove legacy plugin tests#34
quinnj merged 4 commits intomainfrom
jq-remove-legacy-tests

Conversation

@quinnj
Copy link
Copy Markdown
Member

@quinnj quinnj commented Apr 6, 2024

For context, see discussion in JuliaLang/julia#53891

@codecov-commenter
Copy link
Copy Markdown

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
Copy Markdown
Contributor

mkitti commented Apr 7, 2024

Not all of those ciphers are legacy ones...

Comment thread test/runtests.jl
# 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()
Copy link
Copy Markdown

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.

Comment thread test/runtests.jl
EvpBlowFishECB(), # legacy
#EvpBlowFishCFB(), // not supported
EvpBlowFishOFB(), # legacy
EvpAES128CBC(),
Copy link
Copy Markdown

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.

Comment thread test/runtests.jl
end

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

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
Copy Markdown

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

@quinnj quinnj force-pushed the jq-remove-legacy-tests branch from 9734554 to e6cdfd0 Compare April 20, 2024 05:33
@quinnj quinnj merged commit 29cd566 into main Apr 20, 2024
@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