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

test: add invalid config chainWriter tests #454

Merged

Conversation

maximopalopoli
Copy link
Contributor

@maximopalopoli maximopalopoli commented Jan 20, 2025

What Changed?

The goal is to increase the test coverage of chain writer, by testing those cases of passing an invalid config so the contracts addresses are set as nil and the initial validations of the methods will fail.

Increases test coverage of chainio/clients/elcontracts/writer.go up to 79.2%.

Reviewer Checklist

  • Code is well-documented
  • Code adheres to Go naming conventions
  • Code deprecates any old functionality before removing it

@maximopalopoli maximopalopoli marked this pull request as ready for review January 21, 2025 12:50
Copy link
Contributor

@damiramirez damiramirez left a comment

Choose a reason for hiding this comment

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

Left some comments regarding the address values. I think it would be better to use constants instead of literals when we know the addresses come from Anvil.

chainio/clients/elcontracts/writer_test.go Show resolved Hide resolved
chainio/clients/elcontracts/writer_test.go Outdated Show resolved Hide resolved
chainio/clients/elcontracts/writer_test.go Outdated Show resolved Hide resolved
chainio/clients/elcontracts/writer_test.go Outdated Show resolved Hide resolved
chainio/clients/elcontracts/writer_test.go Outdated Show resolved Hide resolved
chainio/clients/elcontracts/writer_test.go Outdated Show resolved Hide resolved
chainio/clients/elcontracts/writer_test.go Outdated Show resolved Hide resolved
chainio/clients/elcontracts/writer_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@MegaRedHand MegaRedHand left a comment

Choose a reason for hiding this comment

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

LGTM

@MegaRedHand MegaRedHand enabled auto-merge January 29, 2025 19:26
@MegaRedHand MegaRedHand added this pull request to the merge queue Jan 29, 2025
Merged via the queue into Layr-Labs:dev with commit 96adf25 Jan 29, 2025
4 checks passed
@MegaRedHand MegaRedHand deleted the test/invalid-config-chain-writer branch January 29, 2025 19:37
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.

3 participants