Skip to content

test(spec): move spec test runners to test folder #699

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

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

mpaulucci
Copy link
Collaborator

Motivation
We shouldn't have test runner code in our lib folder. I understand that we did this because we thought there was no other way, but there is.

@mpaulucci mpaulucci requested a review from a team as a code owner January 30, 2024 16:34
@@ -50,7 +50,6 @@ defmodule LambdaEthereumConsensus.P2P.BlockDownloader do
{:ok, chunks} <- parse_response(response_chunk),
{:ok, blocks} <- decode_chunks(chunks),
:ok <- verify_batch(blocks, slot, count) do
# TODO: handle cases where slot is empty
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unrelated, but this comment doesn't apply anymore.

@@ -12,7 +12,8 @@ defmodule LambdaEthereumConsensus.MixProject do
elixirc_paths: compiler_paths(Mix.env()),
warn_test_pattern: "_remove_warning.exs",
preferred_cli_env: [
dialyzer: :test
dialyzer: :test,
generate_spec_tests: :test
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is the main change, running the task in :test env so that the runners are included as part of the compilation.

Arkenan
Arkenan previously approved these changes Jan 30, 2024
Copy link
Collaborator

@Arkenan Arkenan left a comment

Choose a reason for hiding this comment

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

LGTM

@mpaulucci mpaulucci force-pushed the move-runners-to-test branch from fc1135e to d98a143 Compare January 30, 2024 17:07
@mpaulucci mpaulucci enabled auto-merge (squash) January 30, 2024 17:11
@mpaulucci mpaulucci merged commit f54ba61 into main Jan 30, 2024
@mpaulucci mpaulucci deleted the move-runners-to-test branch January 30, 2024 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants