Do not require a test file in a separately run test case #591
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrykonchin
force-pushed
the
fix-tests-timeout-on-truffleruby-ruby-avoid-requiring-test-file
branch
from
0fabee3
to
9b36ed1
Compare
eregon
reviewed
Jan 3, 2023
| require #{__FILE__.dump} | ||
| include OpenSSL::TestEngine::Utils | ||
| require 'engine_utils' | ||
| include EngineUtils | ||
| #{code} | ||
| end; | ||
| end |
There was a problem hiding this comment.
module Utils below should be removed
There was a problem hiding this comment.
I think we should simply inline these helper methods in Utils. They are both too specialized and don't look very useful anyway.
There was a problem hiding this comment.
I've inlined the helpers.
andrykonchin
requested review from
rhenium and
eregon
and removed request for
rhenium and
eregon
eregon
approved these changes
Jan 4, 2023
test/openssl/test_engine.rb
Outdated
| decrypted = crypt_data(encrypted, key, :decrypt) { OpenSSL::Cipher.new(algo) } | ||
|
|
||
| cipher = engine.cipher(algo) | ||
| cipher.send :encrypt |
There was a problem hiding this comment.
Is the .send necessary? (is it a private method?)
(same below)
There was a problem hiding this comment.
Good catch. Fixed and squashed the commits.
andrykonchin
force-pushed
the
fix-tests-timeout-on-truffleruby-ruby-avoid-requiring-test-file
branch
from
9083cd5
to
ae78467
Compare
rhenium
approved these changes
Jan 4, 2023
andrykonchin
deleted the
fix-tests-timeout-on-truffleruby-ruby-avoid-requiring-test-file
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem:
some tests run test cases in a separate Ruby process, probably to not pollute the global namespace. To share test helpers a test file can be
required in this separate process. So there need additional efforts to prevent auto-launching etc.I propose to not
requirea test file and to move such shared test helpers into a separate place.Actually there are only 2 places where a test file is required
test_engine.rbtest_fips.rbBut only in one place it's really needed (to share test helpers).
Related PR: #589