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

Dysfunctional code on Provider's docs #1355

Closed
arboleya opened this issue Oct 17, 2023 · 3 comments · Fixed by #1493
Closed

Dysfunctional code on Provider's docs #1355

arboleya opened this issue Oct 17, 2023 · 3 comments · Fixed by #1493
Assignees
Labels
bug Issue is a bug docs Requests pertinent to documentation

Comments

@arboleya
Copy link
Member

In this page, the third line of the first code block uses a variable that has never been imported:

const provider = await Provider.create(FUEL_NETWORK_URL);

This is the source:

it('can query address with wallets', async () => {
// #region wallet-query
// #context import { Provider } from 'fuels';
// #context import { generateTestWallet } from '@fuel-ts/wallet/test-utils';
const provider = await Provider.create(FUEL_NETWORK_URL);
const assetIdA = '0x0101010101010101010101010101010101010101010101010101010101010101';
const wallet = await generateTestWallet(provider, [
[42, BaseAssetId],

The fix is simple:

// #context import { Provider, FUEL_NETWORK_URL } from 'fuels';

But I wonder:

  • Don't these #context bits defeat the purpose of having unit-tested docs?
  • Shouldn't these doc-examples.test.ts be placed in the doc-snippets?
@arboleya arboleya added bug Issue is a bug docs Requests pertinent to documentation labels Oct 17, 2023
@arboleya
Copy link
Member Author

cc @danielbate @Torres-ssf — tagging you two since you have been relatively involved on docs land.

@arboleya arboleya changed the title Dysfunctional code on Docs Dysfunctional code on Provider's docs Oct 17, 2023
@Torres-ssf Torres-ssf self-assigned this Oct 17, 2023
@danielbate
Copy link
Member

Definitely agree that doc-examples.test.ts should be moved to doc-snippets, going to reference #1043 as this could be picked up there as part of the wider work to reorganise/recategorise tests.

Don't these #context bits defeat the purpose of having unit-tested docs?
It does however this is a difficult playoff between keeping our snippets clean vs completely unit tested. I investigated trying to keep the actual imports inside the snippet region, then the describe and test would just do an assertion for the code inside the snippet region, essentially ending up with a snippet per file. But I there a few slip ups such as async code where you'd need it at least inside a function block.

Honestly I think my preference is to stay as is until a unit testable solution is found rather than add noise to snippets.

@arboleya
Copy link
Member Author

No worries, but the #context bits perhaps defeating the purpose of having unit-tested docs seem more relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is a bug docs Requests pertinent to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants