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

Remove test command #958

Merged
merged 4 commits into from
Feb 8, 2023
Merged

Remove test command #958

merged 4 commits into from
Feb 8, 2023

Conversation

ascjones
Copy link
Collaborator

@ascjones ascjones commented Feb 8, 2023

Users can use cargo test directly for a better experience.

@ascjones ascjones merged commit a2ff012 into master Feb 8, 2023
@ascjones ascjones deleted the aj/remove-test-command branch February 8, 2023 12:34
@HCastano
Copy link
Contributor

HCastano commented Feb 9, 2023

@ascjones

While I generally agree that using cargo directly is nicer, I don't think we should've remove this so hastily.

This leaves a gap in the user experience since we already have check and build which mirror cargo. It would be natural for a user to expect that cargo contract test also exists.

As a compromise we can have cargo contract test basically just shell to cargo test under the hood.

@ascjones
Copy link
Collaborator Author

ascjones commented Feb 9, 2023

As a compromise we can have cargo contract test basically just shell to cargo test under the hood.

It looks like this is what it was already doing? https://github.com/paritytech/cargo-contract/pull/958/files#diff-f3d6c1267b6d1d54ab15b4fd1870f1802cbb73352881a1652966f7239fe674f0L85

Anyway for whatever reason it was not always working correctly, I wouldn't be averse to having it return if it did work consistently.

This leaves a gap in the user experience since we already have check and build which mirror cargo

Indeed that is a reasonable argument to have it. The counter would be that build and check are doing extra stuff on top of just passing through to cargo, and we don't mirror every cargo subcommand. Although these are the most common.

@HCastano
Copy link
Contributor

Anyway for whatever reason it was not always working correctly, I wouldn't be averse to having it return if it did work consistently.

That's why I assumed it was doing other stuff, because there were issues with it, haha

and we don't mirror every cargo subcommand. Although these are the most common.

Yep, but the lack of it definitely feels awkward to me, and I'm sure users will feel the same way

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.

5 participants