Test failure

classic Classic list List threaded Threaded
17 messages Options
Samuel Mehrbrodt-2 Samuel Mehrbrodt-2
Reply | Threaded
Open this post in threaded view
|

Test failure

Hi,

the tests don't pass after your commit http://cgit.freedesktop.org/libreoffice/core/commit/?id=4a96bc0c0eda8aff6c165bb6a79eb95f2926bb10

Can you fix them please?

Thanks
Samuel

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Jean-Baptiste Faure Jean-Baptiste Faure
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

Le 05/10/2013 22:45, Samuel Mehrbrodt a écrit :
> Hi,
>
> the tests don't pass after your commit
> http://cgit.freedesktop.org/libreoffice/core/commit/?id=4a96bc0c0eda8aff6c165bb6a79eb95f2926bb10

Indeed reverting this commit make the build successful.

Best regards.
JBF

--
Seuls des formats ouverts peuvent assurer la pérennité de vos documents.
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
julien2412 julien2412
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

This post was updated on .
Hi,

If StartColor and EndColor have been reversed, perhaps it was the same for qa part? So what about this patch?
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
index f771ef9..8fdb7fb 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
@@ -733,8 +733,8 @@ void Test::testTextframeGradient()
     uno::Reference<beans::XPropertySet> xFrame(xIndexAccess->getByIndex(0), uno::UNO_QUERY);
     CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_GRADIENT, getProperty<drawing::FillStyle>(xFrame, "FillStyle"));
     awt::Gradient aGradient = getProperty<awt::Gradient>(xFrame, "FillGradient");
-    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xC0504D), aGradient.StartColor);
-    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xD99594), aGradient.EndColor);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xD99594), aGradient.StartColor);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xC0504D), aGradient.EndColor);
     CPPUNIT_ASSERT_EQUAL(awt::GradientStyle_AXIAL, aGradient.Style);

EDIT: just let's try this. At worst, we can revert it.

Julien
Siqi Liu Siqi Liu
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

Hello all, 

Sorry for the late response! This was a patch that I've submitted during the hackathon in Milan and it fixes the #fdo65295 on bugzilla.

Actually we've analyzed the content of the .docx exported by writer and it seems that the startColor and the endColor were accidentally reversed so that each time we import and export a docx with gradient background, we reverse the start and end color in the exported .docx file. 

I would test the patch above to see if that solves the problem when I'm back home. 

Sorry for the broken build :P

ATB,
Siqi



2013/10/6 julien2412 <[hidden email]>
Hi,

If StartColor and EndColor have been reversed, perhaps it was the same for
qa part? So what about this patch?
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
index f771ef9..8fdb7fb 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
@@ -733,8 +733,8 @@ void Test::testTextframeGradient()
     uno::Reference<beans::XPropertySet> xFrame(xIndexAccess->getByIndex(0),
uno::UNO_QUERY);
     CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_GRADIENT,
getProperty<drawing::FillStyle>(xFrame, "FillStyle"));
     awt::Gradient aGradient = getProperty<awt::Gradient>(xFrame,
"FillGradient");
-    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xC0504D), aGradient.StartColor);
-    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xD99594), aGradient.EndColor);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xD99594), aGradient.StartColor);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xC0504D), aGradient.EndColor);
     CPPUNIT_ASSERT_EQUAL(awt::GradientStyle_AXIAL, aGradient.Style);

Julien



--
View this message in context: http://nabble.documentfoundation.org/Test-failure-tp4076788p4076897.html
Sent from the Dev mailing list archive at Nabble.com.
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice



--
--------

Cordialement,
Siqi LIU

Étudiant Ingénieur, 1ère année
École Supérieur d'Électricité (Supélec)


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Siqi Liu Siqi Liu
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

Hello all, 

I've been investigating on this issue and I've tried the patch from Julien and it still doesn't seem to fix the problem. There unit test always fails on the equality assertion even if we reverse that as well. 

Here is what I've got: 

When we check against sal_Int32(0xD99594) (i.e. 14259604 in decimal) for start color aGradient.StartColor 
equality assertion failed
- Expected: 14259604
- Actual  : 12603469

and When we check against sal_Int32(0xC0504D) (i.e. 12603469 in decimal) for start color aGradient.StartColor
equality assertion failed
- Expected: 12603469
- Actual  : 14259604

That is, whenever we reverse the hex value (the expected value) for the unit test, the actual value coming out of the test is surprisingly reversed as well!! Which I failed to explain... 

I have to assume that there are other filters that are tested against this unit test (which in this case needs to reverse the startColor/endColor as I have done in the ooxmlexport filter) or there are some hidden mechanism in this unit test that changes the actual value when we change the expected value... 

My previous patch was tested (on OSX so without unit tests) during the Milano hackathon and it has solved an interoperability bug so presumbly we should keep this patch. Now I guess we need some qa experts to shed some light on the mechanism of this unit test ... 


Cheers, 
Siqi


2013/10/7 Siqi Liu <[hidden email]>
Hello all, 

Sorry for the late response! This was a patch that I've submitted during the hackathon in Milan and it fixes the #fdo65295 on bugzilla.

Actually we've analyzed the content of the .docx exported by writer and it seems that the startColor and the endColor were accidentally reversed so that each time we import and export a docx with gradient background, we reverse the start and end color in the exported .docx file. 

I would test the patch above to see if that solves the problem when I'm back home. 

Sorry for the broken build :P

ATB,
Siqi



2013/10/6 julien2412 <[hidden email]>
Hi,

If StartColor and EndColor have been reversed, perhaps it was the same for
qa part? So what about this patch?
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
index f771ef9..8fdb7fb 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
@@ -733,8 +733,8 @@ void Test::testTextframeGradient()
     uno::Reference<beans::XPropertySet> xFrame(xIndexAccess->getByIndex(0),
uno::UNO_QUERY);
     CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_GRADIENT,
getProperty<drawing::FillStyle>(xFrame, "FillStyle"));
     awt::Gradient aGradient = getProperty<awt::Gradient>(xFrame,
"FillGradient");
-    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xC0504D), aGradient.StartColor);
-    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xD99594), aGradient.EndColor);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xD99594), aGradient.StartColor);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0xC0504D), aGradient.EndColor);
     CPPUNIT_ASSERT_EQUAL(awt::GradientStyle_AXIAL, aGradient.Style);

Julien



--
View this message in context: http://nabble.documentfoundation.org/Test-failure-tp4076788p4076897.html
Sent from the Dev mailing list archive at Nabble.com.
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice



--
--------

Cordialement,
Siqi LIU

Étudiant Ingénieur, 1ère année
École Supérieur d'Électricité (Supélec)




--
--------

Cordialement,
Siqi LIU

Étudiant Ingénieur, 1ère année
École Supérieur d'Électricité (Supélec)


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Efe Gürkan YALAMAN Efe Gürkan YALAMAN
Reply | Threaded
Open this post in threaded view
|

Re: Test failure


2013/10/8 Siqi Liu <[hidden email]>
Hello all, 

Hi,
I've been investigating on this issue and I've tried the patch from Julien and it still doesn't seem to fix the problem. There unit test always fails on the equality assertion even if we reverse that as well. 

Here is what I've got: 

When we check against sal_Int32(0xD99594) (i.e. 14259604 in decimal) for start color aGradient.StartColor 
equality assertion failed
- Expected: 14259604
- Actual  : 12603469

and When we check against sal_Int32(0xC0504D) (i.e. 12603469 in decimal) for start color aGradient.StartColor
equality assertion failed
- Expected: 12603469
- Actual  : 14259604

That is, whenever we reverse the hex value (the expected value) for the unit test, the actual value coming out of the test is surprisingly reversed as well!! Which I failed to explain... 
 
I have to assume that there are other filters that are tested against this unit test (which in this case needs to reverse the startColor/endColor as I have done in the ooxmlexport filter) or there are some hidden mechanism in this unit test that changes the actual value when we change the expected value... 

My previous patch was tested (on OSX so without unit tests) during the Milano hackathon and it has solved an interoperability bug so presumbly we should keep this patch. Now I guess we need some qa experts to shed some light on the mechanism of this unit test ... 



I think the same way too. :)

Please someone correct [me/us] if [I'm/we are] wrong. I definitely have no idea how unit tests written :) This patch written on hackathon to solve a bug as Siqi said.

Tests are written according to the reversed start and end colors(Old code which we changed). After our change at hackathon expected value is wrong because test is written for the old code. So reversing the expected and given HEX values to test should cause reversed values. So maybe it means code is working as expected and the problem is test is written according to reversed values(old code) ?


(Good luck to understand the paragraph above :))
 

Best,

--
Efe Gürkan YALAMAN
http://about.me/efegurkan

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Miklos Vajna-4 Miklos Vajna-4
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

In reply to this post by Siqi Liu
Hi Siqi,

On Tue, Oct 08, 2013 at 08:27:46PM +0200, Siqi Liu <[hidden email]> wrote:
> That is, whenever we reverse the hex value (the expected value) for the
> unit test, the actual value coming out of the test is surprisingly reversed
> as well!! Which I failed to explain...

We have a relatively detailed description of the sw filter test
framework at:

http://opengrok.libreoffice.org/xref/core/sw/qa/extras/README

I guess the issue you hit is that there is an input document, it's
imported, the result is tested against the reference values (1), then
it's exported, imported again and tested a second time (2).

If you only change the exporter and that makes (2) fail; you can't just
change the reference values, because then (1) will fail.

In other words, if you think you need to swap a pair of color values in
the exporter, you probably want to swap them on the import side as well,
otherwise funny things will happen. (The pair will be swapped or not
swapped, depending on if the "save ID" of the save is an odd or an even
number. :) )

The import side of all this is here:

http://opengrok.libreoffice.org/xref/core/oox/source/vml/vmlformatting.cxx#692

Hope this helps,

Miklos

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

signature.asc (205 bytes) Download Attachment
sberg sberg
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

In reply to this post by Siqi Liu
On 10/08/2013 08:27 PM, Siqi Liu wrote:
> My previous patch was tested (on OSX so without unit tests) during the

Just curious, but at least in general there's nothing in our code base
to imply "OSX" means "without unit tests."

Stephan

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Miklos Vajna-4 Miklos Vajna-4
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

On Wed, Oct 09, 2013 at 03:01:02PM +0200, Stephan Bergmann <[hidden email]> wrote:
> Just curious, but at least in general there's nothing in our code
> base to imply "OSX" means "without unit tests."

I was speaking about the Writer filter tests. Those are Linux-only, e.g.:

http://opengrok.libreoffice.org/xref/core/sw/qa/extras/odfimport/odfimport.cxx#43

IIRC the reason for that is --headless not really working on
Windows/Mac, or something like that. I would be glad to remove those
ifdefs, but if the cost is that some tinderbox owners shut their
instance down because of this change, then better not.

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

signature.asc (205 bytes) Download Attachment
Tor Lillqvist-2 Tor Lillqvist-2
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

> IIRC the reason for that is --headless not really working on
> Windows/Mac, or something like that.

But we already have other tests that cheerfully flash windows on and
off for several minutes when one runs  "make" on Windows and OS X.

Or maybe it's only for "make check"? I don't remember, I nowadays tend
to do "make check" whenever I build after some significant amount of
changes anyway. Anyway, I don't think a few more such tests would
hurt.

Maybe those Writer test should be moved to be done only for "make
check", and enabled for all platforms?

--tml
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Miklos Vajna-4 Miklos Vajna-4
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

On Wed, Oct 09, 2013 at 05:32:18PM +0300, Tor Lillqvist <[hidden email]> wrote:
> But we already have other tests that cheerfully flash windows on and
> off for several minutes when one runs  "make" on Windows and OS X.
>
> Or maybe it's only for "make check"? I don't remember, I nowadays tend
> to do "make check" whenever I build after some significant amount of
> changes anyway. Anyway, I don't think a few more such tests would
> hurt.

Right, make check has already this problem, that's the reason e.g.
Norbert's Mac tinderbox doesn't run checks AIUI.

> Maybe those Writer test should be moved to be done only for "make
> check", and enabled for all platforms?

My fear is that ATM if one of those Writer filter checks got broken,
tinderboxes go red, we spam committers, and that's annoying enough that
the problem gets fixed ASAP. For 'make check', this is less true. (I
imagine people who run 'make check' are the same ones who use dbgutil
and werror. :) )

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

signature.asc (205 bytes) Download Attachment
Tor Lillqvist-2 Tor Lillqvist-2
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

>> Maybe those Writer test should be moved to be done only for "make
>> check", and enabled for all platforms?
>
> My fear is that ATM if one of those Writer filter checks got broken,
> tinderboxes go red, we spam committers, and that's annoying enough that
> the problem gets fixed ASAP. For 'make check', this is less true. (I
> imagine people who run 'make check' are the same ones who use dbgutil
> and werror. :) )

Would it be possible to move them to "make check" then for non-Linux,
where they would not cause any new kind of annoyances when running
"make check"? Or would such a change be complicated?

--tml
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
sberg sberg
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

In reply to this post by Tor Lillqvist-2
On 10/09/2013 04:32 PM, Tor Lillqvist wrote:
>> IIRC the reason for that is --headless not really working on
>> Windows/Mac, or something like that.
>
> But we already have other tests that cheerfully flash windows on and
> off for several minutes when one runs  "make" on Windows and OS X.

...not to mention gengal flashing windows at least on Mac OS X during a
plain "make".

Stephan
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
sberg sberg
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

In reply to this post by Miklos Vajna-4
On 10/09/2013 04:44 PM, Miklos Vajna wrote:
> (I imagine people who run 'make check' are the same ones who use
> dbgutil and werror. :)  )

i.e., virtually everybody?
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Michael Stahl-2 Michael Stahl-2
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

In reply to this post by Miklos Vajna-4
On 09/10/13 16:25, Miklos Vajna wrote:

> On Wed, Oct 09, 2013 at 03:01:02PM +0200, Stephan Bergmann <[hidden email]> wrote:
>> Just curious, but at least in general there's nothing in our code
>> base to imply "OSX" means "without unit tests."
>
> I was speaking about the Writer filter tests. Those are Linux-only, e.g.:
>
> http://opengrok.libreoffice.org/xref/core/sw/qa/extras/odfimport/odfimport.cxx#43
>
> IIRC the reason for that is --headless not really working on
> Windows/Mac, or something like that. I would be glad to remove those
> ifdefs, but if the cost is that some tinderbox owners shut their
> instance down because of this change, then better not.

IMHO this disabling of tests based on OS needs to end.

it would be much better to have a configure option
--disable-annoying-tests or whatever so that those who need it can use
it and those who don't can run the tests on any OS.

or perhaps move those tests to subsequentcheck where the other annoying
JUnitTests are.

or actually iirc the PythonTest somehow manages not to pop up any
windows on Windows, perhaps whatever secret it's using to achieve that
could be used elsewhere too.



_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Miklos Vajna-4 Miklos Vajna-4
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

On Wed, Oct 09, 2013 at 07:58:52PM +0200, Michael Stahl <[hidden email]> wrote:
> IMHO this disabling of tests based on OS needs to end.

I just tried enabling the sw filter tests on Mac, but some fixing is
needed there:

https://gerrit.libreoffice.org/#/c/6182/

Possibly only RTF is problematic and the rest can be enabled, but better
to do that interactively, not via trial and error, using gerrit. :)

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

signature.asc (205 bytes) Download Attachment
Miklos Vajna-4 Miklos Vajna-4
Reply | Threaded
Open this post in threaded view
|

Re: Test failure

In reply to this post by sberg
On Wed, Oct 09, 2013 at 04:56:18PM +0200, Stephan Bergmann <[hidden email]> wrote:
> ...not to mention gengal flashing windows at least on Mac OS X
> during a plain "make".

As a start,
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=265c7d62364ab2382491367f66fc14b95681a5a7>
enables all (except one) DOCX filter tests on Mac, if I hear no
problems, I'll continue with enabling other formats as well (as my time
/ limited Mac access allows that).

Miklos

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

signature.asc (205 bytes) Download Attachment