[opensource-dev] Review Request: STORM-977 llmediaplugintest shows up even though -DLL_TESTS:BOOL=OFF has been used

Boroondas Gupte sllists at boroon.dasgupta.ch
Tue Feb 8 10:41:45 PST 2011


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/144/#review347
-----------------------------------------------------------


> If there is some better way to more exactly target these two items please point it out.

You should be able to get the same effect when wrapping the only place where this file is referenced in a LL_TESTS condition, i.e., change https://bitbucket.org/lindenlab/viewer-development/src/b0bceb572090/indra/CMakeLists.txt#cl-74 from
if (NOT LINUX)
to
if (LL_TESTS AND NOT LINUX)
(and the same for the endif, of course)

Whether that's a better place, I don't know.

Though, I think LL_TESTS is the wrong conditional here, anyway. LL_TESTS is for enabling unit and integration tests. llplugintest however, I have learned on IRC today, seems to be a fully separate program based on and similar to uBrowser that could be used to load and test individual llmediaplugins, would it communicate with them in the same way the viewer does. (Which, according to MichelleZ, it doesn't, thus potentially misleading developers of new plugins.)

It should probably not be built unless explicitly requested, thus a new variable, defaulting to OFF and different from LL_TESTS would suit this much better. Or, just delete the referencing at https://bitbucket.org/lindenlab/viewer-development/src/b0bceb572090/indra/CMakeLists.txt#cl-75 completely, thus unconditionally removing it from the dependencies of the viewer itself, and have those that want to build it explicitly state it as a build target.

- Boroondas


On Feb. 8, 2011, 8:46 a.m., Jonathan Yap wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/144/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2011, 8:46 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> llmediaplugintest shows up to be compiled even though -DLL_TESTS:BOOL=OFF has been used on the command line.
> 
> This cmake file does not use the call to LL_ADD_PROJECT_UNIT_TESTS that other cmake files do.  
> 
> LL_ADD_PROJECT_UNIT_TESTS is usually wrapped with if(LL_TESTS)
> 
> I could not figure out which lines suppress the inclusion of copy_plugintest_libs and llmediaplugintest into the list of what is to be built so wrapped the entire file around if(LL_TESTS).  
> 
> If there is some better way to more exactly target these two items please point it out.
> 
> 
> This addresses bug STORM-977.
>     http://jira.secondlife.com/browse/STORM-977
> 
> 
> Diffs
> -----
> 
>   indra/test_apps/llplugintest/CMakeLists.txt b0bceb572090 
> 
> Diff: http://codereview.secondlife.com/r/144/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jonathan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110208/f341d4b5/attachment.htm 


More information about the opensource-dev mailing list