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

Use reflection to generate config whitelist from actual config #852

Merged
merged 1 commit into from
Dec 21, 2018

Conversation

j16r
Copy link
Contributor

@j16r j16r commented Dec 21, 2018

Turns out this code was full of copy and paste glitches. Since this
should ultimately represent a subset of the complete Config, I decided
to fix this issue once and for all with reflection.

Incorrect outputs were:

  1. TX_MIN_CONFIRMATIONS
  2. TASK_MIN_CONFIRMATIONS
  3. DATABASE_POLL_INTERVAL

Turns out this code was full of copy and paste glitches. Since this
should ultimately represent a subset of the complete Config, I decided
to fix this issue once and for all with reflection.

Incorrect outputs were:

  1. TX_MIN_CONFIRMATIONS
  2. TASK_MIN_CONFIRMATIONS
  3. DATABASE_POLL_INTERVAL
Copy link
Contributor

@dimroc dimroc left a comment

Choose a reason for hiding this comment

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

Nice 👍

@j16r j16r merged commit 2b8df30 into master Dec 21, 2018
@j16r j16r deleted the feature/reduce_whitelist_glitches branch December 21, 2018 23:29
asoliman92 pushed a commit that referenced this pull request Jul 31, 2024
## Motivation

To help multiple cases in CCIP-2049 where we expect failures at
different parts of CCIP lifecycle.

## Solution

Make `ValidateRequests` more flexible on what it is able to consider
"Valid"

```go
// Old usage, could only check for specific failures of `ExecStateChanged`
// Should pass
tc.lane.ValidateRequests(true)

// Should fail
tc.lane.ValidateRequests(false)

// New usage, can specify phase, error, and existence
// Should pass
tc.lane.ValidateRequests()

// Should fail
tc.lane.ValidateRequests(ExpectPhaseToFail(ExecStateChanged, ShouldExist()))
```
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.

2 participants