Skip to content

Temporary workaround for ectest.c for [extended tests]#9474

Closed
romen wants to merge 1 commit intoopenssl:masterfrom
romen:temp_fix_ectest
Closed

Temporary workaround for ectest.c for [extended tests]#9474
romen wants to merge 1 commit intoopenssl:masterfrom
romen:temp_fix_ectest

Conversation

@romen
Copy link
Member

@romen romen commented Jul 28, 2019

[extended tests]

This is a temporary workaround for issue #9251, which contains a full
discussion of the real problem.

As a temporary workaround, we test EC_GROUP_new_from_ecparameters()
against a curve that does not currently have alternative
implementations.

The proper fix is dependant on resolution of issue #8615

[extended tests]

This is a temporary workaround for issue openssl#9251, which contains a full
discussion of the real problem.

As a temporary workaround, we test `EC_GROUP_new_from_ecparameters()`
against a curve that does not currently have alternative
implementations.

The proper fix is dependant on resolution of issue openssl#8615
@romen romen added the branch: master Applies to master branch label Jul 28, 2019
@romen romen requested review from mattcaswell and slontis July 28, 2019 13:20
@romen romen self-assigned this Jul 28, 2019
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

Ooops...I meant to hit "approve"

@romen
Copy link
Member Author

romen commented Jul 28, 2019

I will wait for @slontis because originally he wanted to open this, but apparently changed his mind at some point, so maybe he has some feedback to give.

@romen
Copy link
Member Author

romen commented Jul 28, 2019

Failed travis is probably due to #8968

@mattcaswell
Copy link
Member

Failed travis is probably due to #8968

Once this is in, I think we should consider temporarily disabling the external pyca tests until we get a solution to #8968.

Copy link
Member

@slontis slontis left a comment

Choose a reason for hiding this comment

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

I decided not to fix this before, so that the issue might become more visible.

@romen
Copy link
Member Author

romen commented Jul 31, 2019

Merging now!

Merged as f5b7f99

levitte pushed a commit that referenced this pull request Jul 31, 2019
[extended tests]

This is a temporary workaround for issue #9251, which contains a full
discussion of the real problem.

As a temporary workaround, we test `EC_GROUP_new_from_ecparameters()`
against a curve that does not currently have alternative
implementations.

The proper fix is dependant on resolution of issue #8615

Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
(Merged from #9474)
@romen romen closed this Jul 31, 2019
@romen romen deleted the temp_fix_ectest branch July 31, 2019 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch: master Applies to master branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants