Skip to content

Conversation

@edelarua
Copy link
Contributor

Closes #147

@edelarua edelarua added the sme label Oct 16, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2024

Unit Tests Summary

  1 files  111 suites   3m 26s ⏱️
243 tests 243 ✅ 0 💤 0 ❌
513 runs  513 ✅ 0 💤 0 ❌

Results for commit 1c9fce1.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
table_egt01 💔 $1.48$ $+1.80$ $0$ $0$ $0$ $0$
table_egt02 💔 $0.56$ $+1.04$ $0$ $0$ $0$ $0$
table_egt04 💔 $0.25$ $+1.23$ $0$ $0$ $0$ $0$
table_egt05_qtcat 💔 $0.80$ $+1.43$ $0$ $0$ $0$ $0$
table_vst01 💔 $1.04$ $+1.14$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
table_egt01 💔 $1.48$ $+1.80$ EGT01_default_variant_is_produced_correctly
table_egt02 💔 $0.34$ $+1.00$ _EGT02_1._Regardless_of_Abnormality_at_Baseline
table_egt04 💔 $0.25$ $+1.23$ EGT04_default_variant_is_produced_correctly
table_egt05_qtcat 💔 $0.80$ $+1.43$ EGT05_QTCAT_default_variant_is_produced_correctly
table_vst01 💔 $1.04$ $+1.14$ VST01_default_variant_is_produced_correctly

Results for commit 7f78ee5

♻️ This comment has been updated with latest results.

@shajoezhu
Copy link
Contributor

blocked by #159

Signed-off-by: Joe Zhu <[email protected]>
Copy link
Contributor

@shajoezhu shajoezhu left a comment

Choose a reason for hiding this comment

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

hi @edelarua, can you please check these numbers, I think we need to dig into the data a bit more. thank you

@shajoezhu
Copy link
Contributor

hi @edelarua , we are almost there. I was wondering if you can look into this one please https://github.com/insightsengineering/scda.test/actions/runs/11712269992/job/32622560335?pr=154#step:42:187

@edelarua
Copy link
Contributor Author

edelarua commented Nov 6, 2024

hi @edelarua , we are almost there. I was wondering if you can look into this one please https://github.com/insightsengineering/scda.test/actions/runs/11712269992/job/32622560335?pr=154#step:42:187

Hi @shajoezhu,
It seems like the vst01 snapshot is not skipped by the non-CRAN checks like it is by the CRAN checks. I have manually bypassed the skip and regenerated the snapshot so all are passing now!

@shajoezhu
Copy link
Contributor

hi @edelarua , we are almost there. I was wondering if you can look into this one please https://github.com/insightsengineering/scda.test/actions/runs/11712269992/job/32622560335?pr=154#step:42:187

Hi @shajoezhu, It seems like the vst01 snapshot is not skipped by the non-CRAN checks like it is by the CRAN checks. I have manually bypassed the skip and regenerated the snapshot so all are passing now!

this is weird behavior. Thanks @edelarua .

@pawelru I was wondering do you have any insights with why it is skipping one, but not the other?

@shajoezhu
Copy link
Contributor

block this PR by #161

@pawelru
Copy link
Contributor

pawelru commented Nov 7, 2024

snapshot testing are skipped by default for CRAN checks
(on contrary: snapshot tests are enabled for non-CRAN checks)

this is nicely explained here: https://github.com/insightsengineering/idr-tasks/issues/794#issue-2351144387

Copy link
Contributor

@shajoezhu shajoezhu left a comment

Choose a reason for hiding this comment

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

hi @edelarua , we will follow up with the egt table later when new data become available. could you update the snapshot tests, and will merge this PR in. Thanks!

@shajoezhu shajoezhu self-assigned this Nov 14, 2024
@edelarua
Copy link
Contributor Author

hi @edelarua , we will follow up with the egt table later when new data become available. could you update the snapshot tests, and will merge this PR in. Thanks!

@shajoezhu sounds good! The snapshots are up to date as of our last discussion and should be good to go. It seems like the Pkgdown Docs check is still using staged dependencies and so is failing. Is there a way to fix this, or can it be ignored?

@shajoezhu
Copy link
Contributor

hi @edelarua , we will follow up with the egt table later when new data become available. could you update the snapshot tests, and will merge this PR in. Thanks!

@shajoezhu sounds good! The snapshots are up to date as of our last discussion and should be good to go. It seems like the Pkgdown Docs check is still using staged dependencies and so is failing. Is there a way to fix this, or can it be ignored?

thanks @edelarua , let me give it a go

Copy link
Contributor

@shajoezhu shajoezhu left a comment

Choose a reason for hiding this comment

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

lgtm! thanks @edelarua

@edelarua edelarua merged commit 8f5240c into main Nov 14, 2024
29 checks passed
@edelarua edelarua deleted the 147_pv_safety_pt_2@main branch November 14, 2024 23:21
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Explore and test remaining safety can make use of pharmaverseadam part 2

5 participants