Issue identified, but help required! Re: Build failed in Jenkins: Pillar » 30,development,vm #636

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Issue identified, but help required! Re: Build failed in Jenkins: Pillar » 30,development,vm #636

Ben Coman

I have identified the problem with the failing Pillar CI development
builds, but I'm not sure what action should be taken.
"Pillar-ExporterHTML-EstebanLorenzano.47" which is commented as...
"bugfix: PRHTMLWriter>>#writeEmbeddedPicture: should not add '%' to
width property. That should be responsibility of author."

made the following change...
PRHTMLWriter>>writeEmbeddedPicture:
    - anExternalLink parameterAt: 'width' ifPresent: [ :width | img
parameterAt: 'width' put: width greaseString, '%' ].
    + anExternalLink parameterAt: 'width' ifPresent: [ :width | img
parameterAt: 'width' put: width greaseString ].

...which causes PRHTMLWriterTest (PRDocumentWriterTest) >>
testFigureWithWidth
to fail. It can be fixed as follows (plus some debugging instrumentation)...

    PRDocumentWriterTest >> testFigureWithWidth
        | item width |
-       width := '50'.
+       width := '50%'.
        item := PRExternalLink new
            reference: 'file://picture.png';
            embedded: true;
            parameterAt: 'width' put: width;
            yourself.
+      Transcript crShow: self printString ; crShow: (self write: item).
        self assertWriting: item includesText: self widthFor50percents

...but I'm not sure if modifying #testFigureWithWidth is the right thing
to do to fix PRHTMLWriterTest, since this test is inherited by six other
subclasses of PRDocumentWriterTest.  None of the other tests complain,
but that doesn't make it right.

====================
With [width := '50'] running all Pillar tests produces the following
Transcript...

PRLaTeXWriterTest>>#testFigureWithWidth
\begin{figure}
\begin{center}
\includegraphics[width=0.5\textwidth]{picture.png}\caption{file:$/$$/$picture.png.\label{picture.png}}\end{center}
\end{figure}

PRHTMLWriterTest>>#testFigureWithWidth
<figure><img src="picture.png" width="50">
</img><figcaption></figcaption></figure>

PRGitHubMarkdownWriterTest>>#testFigureWithWidth
<a name=""></a><figure><img src="picture.png" width="50%">
</img><figcaption>file://picture.png</figcaption></figure>

PRMarkdownStreamWriterTest>>#testFigureWithWidth
<a name=""></a><figure><img src="picture.png" width="50%">
</img><figcaption>file://picture.png</figcaption></figure>

PRPillarWriterTest>>#testFigureWithWidth
+file://picture.png|width=50+

====================
With [width := '50%'] running all Pillar tests produces the following
Transcript...

PRLaTeXWriterTest>>#testFigureWithWidth
\begin{figure}
\begin{center}
\includegraphics[width=0.5\textwidth]{picture.png}\caption{file:$/$$/$picture.png.\label{picture.png}}\end{center}
\end{figure}

PRHTMLWriterTest>>#testFigureWithWidth
<figure><img src="picture.png" width="50%">
</img><figcaption></figcaption></figure>

PRGitHubMarkdownWriterTest>>#testFigureWithWidth
<a name=""></a><figure><img src="picture.png" width="50%%">
</img><figcaption>file://picture.png</figcaption></figure>

PRMarkdownStreamWriterTest>>#testFigureWithWidth
<a name=""></a><figure><img src="picture.png" width="50%%">
</img><figcaption>file://picture.png</figcaption></figure>

PRPillarWriterTest>>#testFigureWithWidth
+file://picture.png|width=50\%+

====================

Now would existing projects be broken by pictures becoming 50 pixels
wide rather 50% wide ?  If the goal is to be able to specify pixel
width, maybe it would be better to introduce a Pillar parameter
"pixelWidth" .

Presumably the Pillar documentation
https://github.com/pillar-markup/pillar-documentation
would need updating also.

cheers -ben








Reply | Threaded
Open this post in threaded view
|

Re: Issue identified, but help required! Re: Build failed in Jenkins: Pillar » 30,development,vm #636

EstebanLM
yeah, I did not fixed the tests… sorry :(
but I’m completely sure that adding the % is bad bad bad (bad tree times, or, how we like to say in france: très bad :P)

Esteban


On 13 Aug 2014, at 16:54, Ben Coman <[hidden email]> wrote:

>
> I have identified the problem with the failing Pillar CI development builds, but I'm not sure what action should be taken. "Pillar-ExporterHTML-EstebanLorenzano.47" which is commented as...
> "bugfix: PRHTMLWriter>>#writeEmbeddedPicture: should not add '%' to width property. That should be responsibility of author."
>
> made the following change...
> PRHTMLWriter>>writeEmbeddedPicture:
>   - anExternalLink parameterAt: 'width' ifPresent: [ :width | img parameterAt: 'width' put: width greaseString, '%' ].
>   + anExternalLink parameterAt: 'width' ifPresent: [ :width | img parameterAt: 'width' put: width greaseString ].
>
> ...which causes PRHTMLWriterTest (PRDocumentWriterTest) >> testFigureWithWidth
> to fail. It can be fixed as follows (plus some debugging instrumentation)...
>
>   PRDocumentWriterTest >> testFigureWithWidth
>       | item width |
> -       width := '50'.
> +       width := '50%'.
>       item := PRExternalLink new
>           reference: 'file://picture.png';
>           embedded: true;
>           parameterAt: 'width' put: width;
>           yourself.
> +      Transcript crShow: self printString ; crShow: (self write: item).
>       self assertWriting: item includesText: self widthFor50percents
>
> ...but I'm not sure if modifying #testFigureWithWidth is the right thing to do to fix PRHTMLWriterTest, since this test is inherited by six other subclasses of PRDocumentWriterTest.  None of the other tests complain, but that doesn't make it right.
>
> ====================
> With [width := '50'] running all Pillar tests produces the following Transcript...
>
> PRLaTeXWriterTest>>#testFigureWithWidth \begin{figure}
> \begin{center}
> \includegraphics[width=0.5\textwidth]{picture.png}\caption{file:$/$$/$picture.png.\label{picture.png}}\end{center}
> \end{figure}
>
> PRHTMLWriterTest>>#testFigureWithWidth
> <figure><img src="picture.png" width="50">
> </img><figcaption></figcaption></figure>
>
> PRGitHubMarkdownWriterTest>>#testFigureWithWidth <a name=""></a><figure><img src="picture.png" width="50%">
> </img><figcaption>file://picture.png</figcaption></figure>
>
> PRMarkdownStreamWriterTest>>#testFigureWithWidth <a name=""></a><figure><img src="picture.png" width="50%">
> </img><figcaption>file://picture.png</figcaption></figure>
>
> PRPillarWriterTest>>#testFigureWithWidth +file://picture.png|width=50+
>
> ====================
> With [width := '50%'] running all Pillar tests produces the following Transcript...
>
> PRLaTeXWriterTest>>#testFigureWithWidth \begin{figure}
> \begin{center}
> \includegraphics[width=0.5\textwidth]{picture.png}\caption{file:$/$$/$picture.png.\label{picture.png}}\end{center}
> \end{figure}
>
> PRHTMLWriterTest>>#testFigureWithWidth <figure><img src="picture.png" width="50%">
> </img><figcaption></figcaption></figure>
>
> PRGitHubMarkdownWriterTest>>#testFigureWithWidth <a name=""></a><figure><img src="picture.png" width="50%%">
> </img><figcaption>file://picture.png</figcaption></figure>
>
> PRMarkdownStreamWriterTest>>#testFigureWithWidth <a name=""></a><figure><img src="picture.png" width="50%%">
> </img><figcaption>file://picture.png</figcaption></figure>
>
> PRPillarWriterTest>>#testFigureWithWidth +file://picture.png|width=50\%+
>
> ====================
>
> Now would existing projects be broken by pictures becoming 50 pixels wide rather 50% wide ?  If the goal is to be able to specify pixel width, maybe it would be better to introduce a Pillar parameter "pixelWidth" .
>
> Presumably the Pillar documentation https://github.com/pillar-markup/pillar-documentation
> would need updating also.
>
> cheers -ben
>
>
>
>
>
>
>
>