fix scheduled tests

[shajoezhu]

Pull Request

Fixes #nnn

[github-actions]

Unit Tests Summary

  1 files   11 suites   17s ⏱️
110 tests  81 ✅ 29 💤 0 ❌
219 runs  178 ✅ 41 💤 0 ❌

Results for commit 82c4311.

♻️ This comment has been updated with latest results.

[edelarua]

If the test skip check is added to every test in the file you could add it to the top of the file instead so it only needs to check once (this also applies to the other test files)!

Suggested change

	# h_get_diagnostics ----
	skip_if_not_installed("TMB")
	# h_get_diagnostics ----

[shajoezhu]

i am testing this, i think this tmb package should be imported. rather than suggests actually. if true, I will remove all these lines

[danielinteractive]

Do you really need the TMB conditions? mmrm needs TMB of course, but that should be resolved automatically through package dependencies, or not?

[shajoezhu]

import dont work either, after reading this more carefully

Error: processing vignette 'introduction.Rmd' failed with diagnostics:
.onLoad failed in loadNamespace() for 'TMB', details:
  call: dyn.load(file, DLLpath = DLLpath, ...)
  error: unable to load shared object '/github/home/R/x86_64-pc-linux-gnu-library/4.5/TMB/libs/TMB.so':
  libomp.so.5: cannot open shared object file: No such file or directory

it's issue with omp, I will flag this with rhub about there image.

[shajoezhu]

ii also raised an issue at r-hub/containers#85

[pawelru]

Hi. I can see that this is passing without any adjustments to the workflow thus I reverted my changes. This is green now: https://github.com/insightsengineering/tern.mmrm/actions/runs/12907163492/job/35990040546

[pawelru]

It looks like the standard applied in other packages is require(). Both are fully functional and this is just for consistency

Suggested change

	#' @examplesIf requireNamespace("TMB")
	#' @examplesIf require("TMB", quietly = TRUE)

[pawelru]

there is a dedicated function for this - also easier to follow

Suggested change

	testthat::skip_if_not(requireNamespace("TMB"))
	testthat::skip_if_not_installed("TMB")

[shajoezhu]

TMB seem to be installed, but loading was off.

[pawelru]

in such case I would do skip-if-not-installed followed by explicit library to signal that the loading is what we are looking for

[shajoezhu]

tmb is in suggests though, it should implicitly included by mmrm package, and not by us. again, I think this is an image issue. i don't think this pr is good, and it is not solving the real issue

[pawelru]

tmb is in suggests though, it should implicitly included by mmrm package

Then all is needed is just skip if not installed. Following nosuggest logic, the package should remain fully functional without suggested pacakge

and not by us

Partially true - see above. I would say mmrm should specify the version - not here

[shajoezhu]

mmrm has specified it's version. we can ignore that line

[shajoezhu]

Hi. I can see that this is passing without any adjustments to the workflow thus I reverted my changes. This is green now: https://github.com/insightsengineering/tern.mmrm/actions/runs/12907163492/job/35990040546

I m rerunning this on main, and see if it works

[pawelru]

Hey @shajoezhu - I just ran the rhub tests on that branch and it is all passing: https://github.com/insightsengineering/tern.mmrm/actions/runs/13334754421

[shajoezhu]

Hey @shajoezhu - I just ran the rhub tests on that branch and it is all passing: https://github.com/insightsengineering/tern.mmrm/actions/runs/13334754421

hi @pawelru , yes, it is. let's have a catch-up next week. I think the branch's fix is trying to hide the problem rather than solving a real issue, I might be over thinking, but lets have a chat before merging this

[shajoezhu]

good. this PR is not needed. as main is passing. it is not our issue!

[danielinteractive]

yay! one of the many CRAN cases where waiting is best

[shajoezhu]

I agree! @danielinteractive :D