Page MenuHomeFreeBSD

Mk/Uses/kde.mk: Enable testing conditionally
ClosedPublic

Authored by jhale on Jan 21 2021, 5:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 10, 9:13 PM
Unknown Object (File)
Sun, Mar 10, 12:53 PM
Unknown Object (File)
Feb 18 2024, 2:48 PM
Unknown Object (File)
Dec 25 2023, 6:06 PM
Unknown Object (File)
Dec 20 2023, 7:45 AM
Unknown Object (File)
Dec 16 2023, 1:02 AM
Unknown Object (File)
Dec 15 2023, 7:51 PM
Unknown Object (File)
Dec 2 2023, 4:50 PM
Subscribers

Details

Summary
  • Allow autotests if TEST_TARGET is enabled. I would like to keep the TEST option for finance/libalkimia (update pending), but it is not possible if testing is globally disabled.
Test Plan

I've been dogfooding this patch for several months and haven't seen any adverse behavior.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jhale requested review of this revision.Jan 21 2021, 5:06 PM

Moin moin

As someone who got his system wiped twice (before understanding why) by KDE tests that assume that tests are not run as root (and rm -r /etc should fail, ..., fun stuff), I'm slightly opposed to enable tests by default. That's also why it was explicitly turned off here.

But as far as I can see, you only enable them when explicitly setup in the port to do so, right?

Could you commit the cleanup-changes separately, please, nd then refresh the review?

mfg Tobias

Moin moin

As someone who got his system wiped twice (before understanding why) by KDE tests that assume that tests are not run as root (and rm -r /etc should fail, ..., fun stuff), I'm slightly opposed to enable tests by default. That's also why it was explicitly turned off here.

But as far as I can see, you only enable them when explicitly setup in the port to do so, right?

Genau (und tut mir leid)! The autotests would only be run if TEST_TARGET is defined, otherwise they are disabled.

Could you commit the cleanup-changes separately, please, nd then refresh the review?

I can do that.

Could you commit the cleanup-changes separately, please, nd then refresh the review?

I can do that.

Macro makeitso:

This revision was not accepted when it landed; it landed in state Needs Review.Jan 21 2021, 9:14 PM
This revision was automatically updated to reflect the committed changes.

Minor clean-up changes committed. Reopen for review of testing changes.

Update after minor clean-up commit.

I wonder whether it would be better to use a kde_ARGS?

like for example

USES=  kde:5,ctest

and do something like

.     if ${kde_ARGS:Mctest}
CMAKE_ARGS+=	-DBUILD_TESTING:BOOL=true
.    else 
CMAKE_ARGS+=	-DBUILD_TESTING:BOOL=false
.   endif

I wonder whether it would be better to use a kde_ARGS?

like for example

USES=  kde:5,ctest

and do something like

.     if ${kde_ARGS:Mctest}
CMAKE_ARGS+=	-DBUILD_TESTING:BOOL=true
.    else 
CMAKE_ARGS+=	-DBUILD_TESTING:BOOL=false
.   endif

Hmm...IMHO, that seems to be less intuitive. I think it is enough to deliberately define TEST_TARGET in an individual port Makefile to enable the autotests. It is opt-in.

Sure, you're probably right :)

This revision was not accepted when it landed; it landed in state Needs Review.Jan 21 2021, 10:21 PM
This revision was automatically updated to reflect the committed changes.