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

Run tests with more server versions #403

Closed
wants to merge 3 commits into from
Closed

Run tests with more server versions #403

wants to merge 3 commits into from

Conversation

ocheron
Copy link
Contributor

@ocheron ocheron commented Nov 3, 2019

PR to improve the distribution of random values generated in the test suite.
Commit messages give some explanation.

Previously most of the tests had only one value in server
supportedVersions, which is overly restrictive.

This commit merges functions arbitraryPairParams and
arbitraryPairParamsAt to lift this restriction but keep conditions
which are useful.  The filter condition (<= connectVersion) ensures
connectVersion is the common version which is negotiated.
The test case "Groups" used to generate large lists of supported
groups, which made very unlikely to reach the failure scenario
(approx. 5% only).

Limiting the list size increases probability to 25%.
@kazu-yamamoto kazu-yamamoto self-requested a review November 5, 2019 02:20
Copy link
Collaborator

@kazu-yamamoto kazu-yamamoto left a comment

Choose a reason for hiding this comment

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

This code has very good comments!

kazu-yamamoto added a commit to kazu-yamamoto/hs-tls that referenced this pull request Nov 5, 2019
@ocheron
Copy link
Contributor Author

ocheron commented Nov 9, 2019

Thank you for merging. The previous implementation was short and not very clear because not symmetrical. And I think it's better if connectVersion is the real version that is used.
Unfortunately I didn't find a very good descriptive name for listWithOthers.

@ocheron ocheron closed this Nov 9, 2019
@ocheron ocheron deleted the tests-versions branch November 9, 2019 07:42
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.

None yet

2 participants