Skip to content

Conversation

IngelaAndin
Copy link
Contributor

Improvement of existing property based tests

@IngelaAndin IngelaAndin self-assigned this Oct 16, 2025
@IngelaAndin IngelaAndin added the team:PS Assigned to OTP team PS label Oct 16, 2025
@IngelaAndin IngelaAndin requested review from dgud and u3s October 16, 2025 12:45
Copy link
Contributor

github-actions bot commented Oct 16, 2025

CT Test Results

    2 files     66 suites   26m 38s ⏱️
  816 tests   771 ✅  45 💤 0 ❌
4 174 runs  3 308 ✅ 866 💤 0 ❌

Results for commit b9b2135.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@IngelaAndin IngelaAndin requested a review from Copilot October 16, 2025 12:46
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Modernizes property-based tests to use Common Test’s ct_property_test and simplifies property assertions and generators.

  • Replace per-tool conditional includes with ct_property_test.hrl.
  • Switch generators from oneof to elements where selecting from fixed sets.
  • Refactor repeated handshake test logic into a helper and simplify boolean assertions.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
lib/ssl/test/property_test/ssl_eqc_handshake.erl Use ct_property_test.hrl; simplify equality check; switch generators to elements for signature schemes.
lib/ssl/test/property_test/ssl_eqc_ecc.erl Use ct_property_test.hrl in place of per-tool includes.
lib/ssl/test/property_test/ssl_eqc_cipher_format.erl Use ct_property_test.hrl; simplify boolean properties; minor generator/format updates.
lib/ssl/test/property_test/ssl_eqc_chain.erl Use ct_property_test.hrl; factor repeated logic into signature_algs_test/2; use elements for generators.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

_ ->
false
end
tls_handshake:decode_handshake(TLSVersion, Type, Data) == Handshake
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

[nitpick] Prefer exact equality (=:=) over == for term comparison to avoid unintended numeric coercion. Here, using =:= makes the intent of structural equivalence explicit.

Suggested change
tls_handshake:decode_handshake(TLSVersion, Type, Data) == Handshake
tls_handshake:decode_handshake(TLSVersion, Type, Data) =:= Handshake

Copilot uses AI. Check for mistakes.

false
end
).
ssl:str_to_suite(ssl:suite_to_str(CipherSuite)) == CipherSuite).
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

[nitpick] Use exact equality (=:=) instead of == to compare terms. This ensures strict structural equality and avoids numeric type coercion.

Copilot uses AI. Check for mistakes.

).
?FORALL({CipherSuite, _TLSVersion},
?LET(Version, pre_tls_1_3_version(), {anon_cipher_suite(Version), Version}),
ssl:str_to_suite(ssl:suite_to_str(CipherSuite)) == CipherSuite
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

[nitpick] Use =:= instead of == for exact term equality to make the comparison strict and unambiguous.

Suggested change
ssl:str_to_suite(ssl:suite_to_str(CipherSuite)) == CipherSuite
ssl:str_to_suite(ssl:suite_to_str(CipherSuite)) =:= CipherSuite

Copilot uses AI. Check for mistakes.

@IngelaAndin IngelaAndin merged commit 9efd029 into erlang:maint Oct 20, 2025
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team:PS Assigned to OTP team PS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants