Skip to content

fix(e2e): viper errors with invalid config and genesis manifest keys#4016

Merged
cason merged 16 commits intomainfrom
cason/e2e-viper-fix
Sep 10, 2024
Merged

fix(e2e): viper errors with invalid config and genesis manifest keys#4016
cason merged 16 commits intomainfrom
cason/e2e-viper-fix

Conversation

@cason
Copy link
Copy Markdown

@cason cason commented Sep 6, 2024

Addresses comments #3969 (review) and #3967 (review) on previous PRs.

The solution is in fact simple, as it is possible to tell viper to return an error if one of the configured entries (key/value pairs) were not applied to the destination structure, because they don't exist or are invalid.


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments

@cason cason requested a review from a team as a code owner September 6, 2024 08:16
@cason cason requested a review from a team September 6, 2024 08:16
Copy link
Copy Markdown
Collaborator

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

@andynog andynog added the e2e Related to our end-to-end tests label Sep 6, 2024
@andynog andynog added this to the 2024-Q3 milestone Sep 6, 2024
Copy link
Copy Markdown
Collaborator

@andynog andynog left a comment

Choose a reason for hiding this comment

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

lgtm, I've done some testing and other than 1 suggestion I think it's all good.

One thing I've noticed is that if there's an error generated because of the parsing errors in the config section of the manifest (e.g. invalid key), the testnet files are still generated (except the ones in the config folder). But that's a different scope and could be something to improve later.

Thanks !

@cason cason enabled auto-merge September 9, 2024 13:58
@cason cason added this pull request to the merge queue Sep 10, 2024
Merged via the queue into main with commit bff1667 Sep 10, 2024
@cason cason deleted the cason/e2e-viper-fix branch September 10, 2024 16:02
mergify bot pushed a commit that referenced this pull request Oct 20, 2024
…eys (#4016)

Addresses comments
#3969 (review)
and
#3967 (review)
on previous PRs.

The solution is in fact simple, as it is possible to tell viper to
return an error if one of the configured entries (key/value pairs) were
not applied to the destination structure, because they don't exist or
are invalid.

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
(cherry picked from commit bff1667)

# Conflicts:
#	test/e2e/pkg/testnet.go
#	test/e2e/runner/setup.go
cason pushed a commit that referenced this pull request Oct 20, 2024
…eys (#4016)

Addresses comments
#3969 (review)
and
#3967 (review)
on previous PRs.

The solution is in fact simple, as it is possible to tell viper to
return an error if one of the configured entries (key/value pairs) were
not applied to the destination structure, because they don't exist or
are invalid.

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
cason pushed a commit that referenced this pull request Oct 20, 2024
…eys (backport #4016) (#4313)

Addresses comments
#3969 (review)
and
#3967 (review)
on previous PRs.

The solution is in fact simple, as it is possible to tell viper to
return an error if one of the configured entries (key/value pairs) were
not applied to the destination structure, because they don't exist or
are invalid.

---

This is an manual backport of pull request
#4016.

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2e Related to our end-to-end tests

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants