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: make tests work with new kilt node cli #796

Merged
merged 4 commits into from
Aug 22, 2023
Merged

Conversation

rflechtner
Copy link
Contributor

@rflechtner rflechtner commented Aug 21, 2023

fixes KILTProtocol/ticket#2835

A different startup strategy is required for kilt nodes based on polkadot v0.9.43 because the CLI has changed and arguments have been renamed.
This introduces a fallback strategy if the container can't be started, and will try again using a set of arguments which should work for the new CLI.

How to test:

Latest currently uses the old CLI, while latest-develop uses the new CLI. If both integration test runs succeed, this works.

Checklist:

  • I have verified that the code works
  • I have verified that the code is easy to understand
    • If not, I have left a well-balanced amount of inline comments
  • I have left the code in a better state
  • I have documented the changes (where applicable)
    • Either PR or Ticket to update the Docs
    • Link the PR/Ticket here

Sorry, something went wrong.

@rflechtner rflechtner marked this pull request as ready for review August 21, 2023 21:54
@rflechtner rflechtner mentioned this pull request Aug 21, 2023
@rflechtner rflechtner requested a review from Ad96el August 21, 2023 22:11
Copy link
Member

@Ad96el Ad96el left a comment

Choose a reason for hiding this comment

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

Two minor comments.

Do we not have knowledge of how the metadata shape when starting the node? Otherwise, could we utilize the version in the metadata to determine which arguments are required or not?

@rflechtner
Copy link
Contributor Author

Two minor comments.

Do we not have knowledge of how the metadata shape when starting the node? Otherwise, could we utilize the version in the metadata to determine which arguments are required or not?

We don't, as we get the metadata from the node. You probably know more about the node CLI than me, and if we can use it to export the metadata, but using the CLI to avoid breaking changes in the CLI is probably a little circular?

@rflechtner rflechtner requested a review from Ad96el August 22, 2023 10:17
Copy link
Member

@Ad96el Ad96el left a comment

Choose a reason for hiding this comment

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

Yeah, I was thinking that the metadata comes from the started container node but was not sure. LGTM

@rflechtner rflechtner merged commit 26751e6 into develop Aug 22, 2023
@rflechtner rflechtner deleted the rf-new-node-cli branch August 22, 2023 11:31
rflechtner added a commit that referenced this pull request Aug 22, 2023
* test: make tests work with new kilt node cli
* test: re-use integration test utils in bundle tests
* test: bump runtime version in integration test switch condition
rflechtner added a commit that referenced this pull request Aug 22, 2023
* refactor!: nest ctype in ICTypeDetails (#766)
* refactor: change ctype parameter name to cType where possible

* feat: re-export imported types from @kiltprotocol/types (#762)
* feat: deprecate CType meta schema draft-01 (#778)

* fix: default to log level warn in non-production environments

* test: skip expensive delegation node tests
* test: esm variant of integration tests (#779)
* test: make tests work with new kilt node cli (#796)
* chore: update metadata to 1.11.0
* chore(deps): update cbor-web to v9
* chore(deps-dev): bump webpack from 5.70.0 to 5.76.0 (#771)
* chore(deps-dev): bump semver-regex from 3.1.3 to 3.1.4 (#775)
* chore(deps-dev): bump typedoc to resolve dependency conflict
* ci: make latest-develop tests optional and latest mandatory

Co-authored-by: Gerawork Aynekulu <ggera@users.noreply.github.com>
Co-authored-by: Raphael Flechtner <raphael@kilt.io>
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.

None yet

2 participants