[LyX/master] ctests: fix race condition for parallel testing

[Scott Kostyshak]

On Mon, Nov 16, 2020 at 03:53:46PM +0100, Kornel Benko wrote:
> Am Mon, 16 Nov 2020 09:38:09 -0500
> schrieb Scott Kostyshak <skostysh at lyx.org>:
> 
> > On Mon, Nov 16, 2020 at 12:26:57PM +0100, Kornel Benko wrote:
> > > Am Sun, 15 Nov 2020 20:44:41 -0500
> > > schrieb Scott Kostyshak <skostysh at lyx.org>:
> > >   
> > > > On Mon, Nov 16, 2020 at 12:10:14AM +0100, Kornel Benko wrote:  
> > > > > Am Sun, 15 Nov 2020 23:48:55 +0100
> > > > > schrieb Kornel Benko <kornel at lyx.org>:
> > > > >     
> > > > > > Am Sun, 15 Nov 2020 23:10:48 +0100 (CET)
> > > > > > schrieb Scott Kostyshak <skostysh at lyx.org>:
> > > > > >     
> > > > > > > commit dbb72a370aa8117c7dee645b195664115a7b70ed
> > > > > > > Author: Scott Kostyshak <skostysh at lyx.org>
> > > > > > > Date:   Sun Nov 15 17:40:02 2020 -0500
> > > > > > > 
> > > > > > >     ctests: fix race condition for parallel testing
> > > > > > >     
> > > > > > >     The unicode tests would often fail when tested in parallel because
> > > > > > >     we were not exporting to unique file names. From what I understand,
> > > > > > >     a variant similar to the following race condition occurred:
> > > > > > >     
> > > > > > >     1. Thread A exports to file blah.pdf.
> > > > > > >     2. Thread B exports to file blah.pdf.
> > > > > > >     3. Thread A confirms file blah.pdf exists.
> > > > > > >     4. Thread A deletes exported file blah.pdf to clean up.
> > > > > > >     5. Thread B fails to find file blah.pdf and reports a failure.
> > > > > > > ---
> > > > > > >  development/autotests/export.cmake |    4 +++-
> > > > > > >  1 files changed, 3 insertions(+), 1 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/development/autotests/export.cmake
> > > > > > > b/development/autotests/export.cmake index d2a0356..1384800 100755
> > > > > > > --- a/development/autotests/export.cmake
> > > > > > > +++ b/development/autotests/export.cmake
> > > > > > > @@ -74,7 +74,9 @@ if(format MATCHES "dvi|pdf")
> > > > > > >    if(NOT _erg)
> > > > > > >      message(FATAL_ERROR "Export failed while converting")
> > > > > > >    endif()
> > > > > > > -  set(result_file_name ${file}_${_ft}.${extension})
> > > > > > > +  # We only need "_${ENCODING}" for unicode tests (because multiple encodings
> > > > > > > +  # are tested with the same format), but doesn't hurt to include for all.
> > > > > > > +  set(result_file_name ${file}_${_ft}_${ENCODING}.${extension})
> > > > > > >  else()
> > > > > > >    message(STATUS "Converting with perl ${Perl_Script}")
> > > > > > >    set(LYX_SOURCE "${TempDir}/${file}.lyx")      
> > > > > > 
> > > > > > GOOD CATCH :)
> > > > > > 
> > > > > > 	Kornel    
> > > > > 
> > > > > OTOH, it should not matter, because each test runs in its own temporary directory
> > > > > ${TempDir}.
> > > > > (See export.cmake:40, 42)    
> > > > 
> > > > Currently I don't think it is. If you comment out the following line (around 324):
> > > > 
> > > >   file(REMOVE ${result_file_name})
> > > > 
> > > > and run the following:
> > > > 
> > > >   ctest -R "examples/Welcome_pdf2"
> > > > 
> > > > I then see the following file created:
> > > > 
> > > >   ./autotests/out-home/examples/Welcome_defaultF.pdf2  
> > > 
> > > I see (:, don't know why I did it that way ... maybe to omit spaces in the paths or
> > > the path-length should not be too long ...
> > >   
> > > > Maybe the following change would fix this?
> > > > 
> > > >   -  set(result_file_name ${file}_${_ft}_${ENCODING}.${extension})
> > > >   +  set(result_file_name ${TempDir}/${file}_${_ft}_${ENCODING}.${extension})
> > > > 
> > > > If so please commit.  
> > > 
> > > Or use the tempdir correctly ...
> > >   
> > > > Scott  
> > > 
> > > Tried to use tempdir at 53e064ce.  
> > 
> > Looks good, thanks. We could remove ${ENCODING} from result_file_name now if you
> > prefer. 
> 
> I does not hurt.
> 
> > I don't have a preference. If we remove it I suppose we should remove ${_ft}
> > also to make it a bit more clear that the base name of the file is not unique (i.e.,
> > that we rely on being in a temporary directory).
> 
> Yes, but we would loss the hint of what was done to get this results (in case we keep the
> tempdir (with env LYX_DEBUG_LATEX=1)), because the tempdir name itself gives no hints.
> And posting (if needed) would make it also clear, how the respective file was created.

Ah that is a good point. That is indeed helpful especially when
debugging. Let's keep things as they are then. I do not think we will
have problems because of a "too long" filename. These days I think it is
possible to have very long file names with no problem.

Scott

> OTOH, the associated log has enough infos, so I am not hard argumenting.
> 
> > Scott
> 
> 	Kornel



> -- 
> lyx-devel mailing list
> lyx-devel at lists.lyx.org
> http://lists.lyx.org/mailman/listinfo/lyx-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20201116/52bc4890/attachment.asc>