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

1682 fix init test #1738

Merged
merged 4 commits into from
Dec 20, 2024
Merged

1682 fix init test #1738

merged 4 commits into from
Dec 20, 2024

Conversation

Ifropc
Copy link
Contributor

@Ifropc Ifropc commented Nov 18, 2024

What

As part of #1682 fix init test.
Moves init_and_deploy into ./integration as it uses quickstart image to deploy.
For existing init test allow to use -rc SDK if our current version is -rc itself.
Update template soroban SDK version to 22-rc3

Why

Test fixes

Known limitations

Some unrelated tests fail, see #1682

Copy link
Contributor

@elizabethengelman elizabethengelman left a comment

Choose a reason for hiding this comment

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

:shipit:

@@ -5,7 +5,7 @@ members = [
]

[workspace.dependencies]
soroban-sdk = "21.0.0"
soroban-sdk = "22.0.0-rc.3"
Copy link
Member

Choose a reason for hiding this comment

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

The CLI should probably only ever generate templates for stable releases, not release candidates. Even if folks are using an RC of the CLI it's unclear to me it should encourages folks to adopt other unstable software releases in other areas. Can we keep this as the stable?

Suggested change
soroban-sdk = "22.0.0-rc.3"
soroban-sdk = "22.0.0"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this check was more for our development process, but if we are fine with tests temporarily failing in dev once we bump the version, then I agree we should make it always stable

Copy link
Member

Choose a reason for hiding this comment

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

Why would the test fail? I'm suggesting that the init function should generate from a template that is always the latest stable, so that might mean that 22-rc should be initing with an sdk that is 21.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see what you mean. Previously test was failing because cli version is 22-rc and test checks that the template uses same major version. Should we keep that condition (test will fail on rc though) or maybe allow for -rc to use previous stable version?

Comment on lines +24 to +27
if is_rc {
sdk_version.and_then(|x| x.split('-').next()) == Some(&format!("{major}.0.0"))
} else {
sdk_version == Some(&format!("{major}.0.0"))
Copy link
Member

Choose a reason for hiding this comment

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

I think we should write an assertion that the generated template is always a stable, non-rc version. See other comment.

@Ifropc Ifropc merged commit a664025 into 1682-help-tests Dec 20, 2024
18 of 31 checks passed
@Ifropc Ifropc deleted the 1682-init-test branch December 20, 2024 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants