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

chore: generate unit test code #15

Merged
merged 1 commit into from
Jul 23, 2024
Merged

Conversation

quake
Copy link
Contributor

@quake quake commented Jul 22, 2024

No description provided.

@quake quake requested review from xxuejie and XuJiandong July 22, 2024 01:59
@xxuejie
Copy link
Collaborator

xxuejie commented Jul 22, 2024

Why not update tests/src/tests.rs file directly?

@quake
Copy link
Contributor Author

quake commented Jul 22, 2024

Why not update tests/src/tests.rs file directly?

cargo generate cannot work on the final destination folder, we have to use a script to modify this file.

@xxuejie
Copy link
Collaborator

xxuejie commented Jul 22, 2024

Why not update tests/src/tests.rs file directly?

cargo generate cannot work on the final destination folder, we have to use a script to modify this file.

I mean in the pull request, we can update tests/src/tests.rs file directly to contain the unit test. There is no need to generate a template using a previous tests/src/tests.rs file, then update it later with another file.

@quake
Copy link
Contributor Author

quake commented Jul 22, 2024

I mean in the pull request, we can update tests/src/tests.rs file directly to contain the unit test. There is no need to generate a template using a previous tests/src/tests.rs file, then update it later with another file.

I want to generate unit test per contract automatically

fn test_{{crate_name}}() {
    // deploy contract
    let mut context = Context::default();
    let contract_bin: Bytes = Loader::default().load_binary("{{project-name}}");

@xxuejie
Copy link
Collaborator

xxuejie commented Jul 22, 2024

Ah I see what this means here, but to me that brings more questions:

  • Shall we put this test in each individual contract crate, or shall we put this in the tests folder for the global workspace? Feels like this is a question we haven't discussed.
  • The code right now only deals with contract template, but we also have templates for other contracts(c-wrapper-crate, atomics-contract, etc.), those additional templates have not been taken care of. So maybe a better solution here, is to put the unit test snippet at the workspace level? For example, we can have a snippets folder in the root of workspace, and we can simply create the tests needed from files in this snippets folder.

@quake
Copy link
Contributor Author

quake commented Jul 22, 2024

Shall we put this test in each individual contract crate, or shall we put this in the tests folder for the global workspace? Feels like this is a question we haven't discussed.

I prefer put test in individual contract folder, however, capsule and script-templates put it in the global workspace, I think it's better to follow the previous convention.

The code right now only deals with contract template, but we also have templates for other contracts(c-wrapper-crate, atomics-contract, etc.)

I haven't looked the detail of these template tests, are they the same as regular contract tests? If they are, I can add them as your suggestion.

@xxuejie
Copy link
Collaborator

xxuejie commented Jul 22, 2024

I haven't looked the detail of these template tests, are they the same as regular contract tests? If they are, I can add them as your suggestion.

They are the same contract template you can add to the workspace, with varying options.

@xxuejie
Copy link
Collaborator

xxuejie commented Jul 22, 2024

I prefer put test in individual contract folder, however, capsule and script-templates put it in the global workspace, I think it's better to follow the previous convention.

Personally, I don't think following existing conventions here is that important, we are already introducing new tool with different commands.

@quake
Copy link
Contributor Author

quake commented Jul 22, 2024

For example, we can have a snippets folder in the root of workspace, and we can simply create the tests needed from files in this snippets folder.

I tried to change it this way, but I found that cargo generate can't read files from the current workspace directory unless we change it with sed, but that makes the shell script complicated, so I went with the easy way: put a test-generate.rs file in each template directory. Please help to review again, thanks.

@xxuejie
Copy link
Collaborator

xxuejie commented Jul 22, 2024

I understand the cause here, and I'm fine with the solution, but at least can we move all those test-generate.rs files at the root of each template to a designated folder? For example, we can have:

  • contract/_to_remove/test-generate.rs
  • stack-reorder-contract/_to_remove/test-generate.rs
  • atomics-contract/_to_remove/test-generate.rs

This way at least we know the files in _to_remove are only there for template creation purposes, and will be expected to be removed later. We can also reuse the same folder for similar purposes in the future.

@xxuejie
Copy link
Collaborator

xxuejie commented Jul 22, 2024

And one more thing: it possible to generate a single contract as a repo, instead of using a workspace, do you think it makes sense to add a default unit test for the single standalone contract here: https://github.com/cryptape/ckb-script-templates/tree/main/standalone-contract?

@quake quake force-pushed the quake/unit-test branch from 713e976 to da22dbe Compare July 23, 2024 00:56
@quake
Copy link
Contributor Author

quake commented Jul 23, 2024

I understand the cause here, and I'm fine with the solution, but at least can we move all those test-generate.rs files at the root of each template to a designated folder? For example, we can have:

  • contract/_to_remove/test-generate.rs
  • stack-reorder-contract/_to_remove/test-generate.rs
  • atomics-contract/_to_remove/test-generate.rs

This way at least we know the files in _to_remove are only there for template creation purposes, and will be expected to be removed later. We can also reuse the same folder for similar purposes in the future.

Added a folder .cargo_generate

And one more thing: it possible to generate a single contract as a repo, instead of using a workspace, do you think it makes sense to add a default unit test for the single standalone contract here: https://github.com/cryptape/ckb-script-templates/tree/main/standalone-contract?

Resolved

Please help to review again, thanks.

@quake quake merged commit c176286 into cryptape:main Jul 23, 2024
9 checks passed
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.

3 participants