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: Fix up doc tests and run them on CI #1057

Open
wants to merge 3 commits into
base: next
Choose a base branch
from
Open

Conversation

igamigo
Copy link
Collaborator

@igamigo igamigo commented Jan 9, 2025

No description provided.

@igamigo igamigo requested review from Mirko-von-Leipzig and PhilippGackstatter and removed request for Mirko-von-Leipzig January 9, 2025 22:18
@igamigo igamigo added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Jan 9, 2025
Copy link
Contributor

@PhilippGackstatter PhilippGackstatter left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Thanks for fixing the mock chain ones as well, I hadn't seen those. I think I was running without --testing.

/// .unwrap();
/// mock_chain.seal_block(None);
/// let tx_context = mock_chain.build_tx_context(sender.id(), &[note.id()], &[]).build();
/// // tx_context.execute();
Copy link
Contributor

Choose a reason for hiding this comment

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

If we can't make it a full example and have to comment this line, should we just add no_run (https://doc.rust-lang.org/rustdoc/write-documentation/documentation-tests.html#attributes)? That way it's compiled but not executed which seems fine here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah yes, that sounds good.

Comment on lines 179 to 185
/// let template = AccountComponentTemplate::new(component_template, library);
///
/// let component = AccountComponent::from_template(&template, &init_storage_data)?;
/// # Ok(())
/// # }
/// ```

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about moving this example to the type-level, e.g. on AccountComponentMetadata directly for more visibility?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed!

.github/workflows/test.yml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog This PR does not require an entry in the `CHANGELOG.md` file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants