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

Improve Pallet UI doc test #5264

Merged
merged 5 commits into from
Aug 6, 2024
Merged

Improve Pallet UI doc test #5264

merged 5 commits into from
Aug 6, 2024

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Aug 6, 2024

Test currently failing, therefore improving to include a file from the same crate to not trip up the caching.

R0 silent since this is only modifying unpublished crates.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez requested a review from a team as a code owner August 6, 2024 14:35
@ggwpez ggwpez added the R0-silent Changes should not be mentioned in any release notes label Aug 6, 2024
Copy link
Contributor

@seadanda seadanda left a comment

Choose a reason for hiding this comment

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

Sorry 😅

substrate/frame/support/test/example-pallet-doc.md Outdated Show resolved Hide resolved
substrate/frame/support/test/tests/pallet.rs Outdated Show resolved Hide resolved
substrate/frame/support/test/tests/pallet.rs Outdated Show resolved Hide resolved
@athei
Copy link
Member

athei commented Aug 6, 2024

The test was flaky because it became out of sync with the README it was testing against?

ggwpez and others added 3 commits August 6, 2024 17:25
Co-authored-by: Dónal Murray <donal.murray@parity.io>
Co-authored-by: Dónal Murray <donal.murray@parity.io>
Co-authored-by: Dónal Murray <donal.murray@parity.io>
@ggwpez
Copy link
Member Author

ggwpez commented Aug 6, 2024

The test was flaky because it became out of sync with the README it was testing against?

CI/CD team is investigating if its a caching issue since it included a file from a different crate (which is possibly incorrectly cached). But it should have normally not failed. This MR just fixes the behaviour of using a real readme and uses testing readmes from the same crate instead.

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 2/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6913794

@seadanda
Copy link
Contributor

seadanda commented Aug 6, 2024

Interesting, my grammar nits have accidentally repeated the issue but within the same package. Looks like the docs cache is refreshed but not the cache of the package itself (if I'm interpreting the diff properly (?))

@ggwpez
Copy link
Member Author

ggwpez commented Aug 6, 2024

Interesting, my grammar nits have accidentally repeated the issue but within the same package. Looks like the docs cache is refreshed but not the cache of the package itself (if I'm interpreting the diff properly (?))

Yea, or the CI was started in between two commits where i applied your suggestions. Going to restart and wait.

PS: Another typo

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added this pull request to the merge queue Aug 6, 2024
Merged via the queue into master with commit 291c082 Aug 6, 2024
159 of 160 checks passed
@ggwpez ggwpez deleted the oty-ui-test-readme branch August 6, 2024 18:37
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Aug 28, 2024
Test currently failing, therefore improving to include a file from the
same crate to not trip up the caching.

R0 silent since this is only modifying unpublished crates.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Dónal Murray <donal.murray@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants