LibreOffice uses some automation to compile a list of bug reports that have been fixed, but are missing a test. Writing a test for a bug fix in this list is a great way to contribute to the LibreOffice open source project.
A successfully merged test is run in the Continuous Integration pipeline via Jenkins each time a patch is uploaded to Gerrit. This ensures that the original bug is not introduced back into the project in the form of a regression. Below we’ll go through some steps to write a test from this list.
Not all bugs and bug reports are created equal, so finding a good candidate is crucial to successfully writing a test. Here are some things to think about when looking through bug reports.
- Find a bug that is obvious to see, a trivial example of this is a bug that causes LibreOffice to crash. It should be obvious to test for a crash, versus testing for a subtle layout issue.
- Look for a bug report that is either very new, or where the bug fix is a limited number of lines of code.
- Look for clear steps to reproduce the bug.
- It is helpful if the bug fix is confirmed in the bug report. If there is no confirmation, the first thing you should do is run LibreOffice and use the steps listed in the bug report to make sure that the bug is indeed fixed.
We are going to use the bug report from tdf#61000 as an example to go through the process of writing a test from the list. A link to the report can be found below the image.
https://bugs.documentfoundation.org/show_bug.cgi?id=61000
Looking at the top comment, you can see that the bug is causing LibreOffice to crash when the attached file is opened. There are clear steps to reproduce the bug in comment 1, and the bug has been confirmed in comment 3.
Using the commit found in comment 8 of the bug report, we can use git to show the bug fix.
git show dbb74ee9950dc706ea4fde3397a4c1d19b172fa9
commit dbb74ee9950dc706ea4fde3397a4c1d19b172fa9 Author: Michael Stahl <mstahl@redhat.com> Date: Wed Feb 27 23:20:24 2013 +0100fdo#61000: writerfilter: filter out unsupported list tab stops Change-Id: Ic9d31eba84b1d8d9cf93d8289621a65d43521a8b
diff --git a/writerfilter/source/dmapper/NumberingManager.cxx b/writerfilter/source/dmapper/NumberingManager.cxx index fa59bd2c7062..38b1f07a1c9d 100644 --- a/writerfilter/source/dmapper/NumberingManager.cxx +++ b/writerfilter/source/dmapper/NumberingManager.cxx @@ -135,7 +135,13 @@ void ListLevel::SetValue( Id nId, sal_Int32 nValue ) m_nXChFollow = nValue; break; case NS_ooxml::LN_CT_TabStop_pos: - m_nTabstop = nValue; + if (nValue < 0) + { + SAL_INFO("writerfilter", + "unsupported list tab stop position " << nValue); + } + else + m_nTabstop = nValue; break; default: OSL_FAIL( "this line should never be reached");
We can see that reverting the bug fix is straight forward. The lines that were added can be commented out and the original line that was deleted can be added back to the code.
If you try to open the file listed in the commit to do the edit, you will find that the file is not there. When looking at old commits this can happen, sometimes files get moved, so we can search for the relevant function with
git grep "ListLevel::SetValue"
to get
sw/source/writerfilter/dmapper/NumberingManager.cxx:void ListLevel::SetValue( Id nId, sal_Int32 nValue )
which shows us that the function in question now resides in
sw/source/writerfilter/dmapper/NumberingManager.cxx
Having found the correct file to edit, we get
break;
case NS_ooxml::LN_CT_TabStop_pos:
m_nTabstop = nValue;
#if (nValue < 0)
#{
#SAL_INFO("writerfilter",
#"unsupported list tab stop position " << nValue);
#}
#else
#m_nTabstop = nValue;
break;
default:
OSL_FAIL( "this line should never be reached");
The next step is to build with the bug fix reverted and test that the bug is now present. Testing for the bug is simple in this case, just download attachment 1 from the bug report and try to open it with the bug fix reverted.
Once we are sure the bug is reintroduced, it is a good idea to create a branch in git and commit the revert. In this way, you will be able to switch between having the bug present, and not present. Once you have your top commit as the bug fix revert, you can use
git checkout HEAD~
to go back one commit, where the bug is fixed and
git switch -
to go back to the bug being reintroduced. In this way you can easily and quickly switch the behavior of LibreOffice in order to test, if your test 😆, is working correctly.
Now we need to decide where our test will reside. Since the bug shows when using writer, we will look in sw/qa/extras
accessibility globalfilter mailmerge pdf tiledrendering2 unowriter
autocorrect htmlexport odfexport README txtencexport ww8export
data htmlimport odfimport rtfexport txtexport ww8import
docbookexport indexing ooxmlexport rtfimport txtimport
fodfexport layout ooxmlimport tiledrendering uiwriter
The README file is very helpful here, I’d encourage anyone writing a test to read it, it contains a lot of helpful information. When deciding where to put a test, a few ideas to keep in mind are:
- Take note of the file or area of files the bug fix resides in. In this case, source/writerfilter is a clue that this is an export bug.
- What file type are we dealing with? A problem opening up a .docx file would suggest that the test may go in ooxmlexport.
- Look for similar tests. Sometimes it can be helpful to look through some of the test suites to see if there are other similar tests.
In this case we’re dealing with a crash when opening a .docx file. This points to ooxmlexport as a good place to put the test.
The last part of the process is to actually write the test. In order to test for a crash, we just open the file and see if LibreOffice crashes.
CPPUNIT_TEST_FIXTURE(Test, testTdf61000)
{
// Without the fix in place this crashes on opening
loadAndSave("tdf61000.docx");
xmlDocUniquePtr pXmlDoc = parseExport(u"word/numbering.xml"_ustr);
assertXPath(
pXmlDoc,
"//w:numbering/w:abstractNum[@w:abstractNumId='1']/w:lvl[@w:ilvl='0']/w:numFmt"_ostr,
"val"_ostr, u"bullet"_ustr);
// Without the fix in place, this would be -540, and the abstractNumId is 4
// The negative value of the tab stop is the culprit for the crash
assertXPath(
pXmlDoc,
"//w:numbering/w:abstractNum[@w:abstractNumId='1']/w:lvl[@w:ilvl='0']/w:pPr/w:tabs/w:tab"_ostr,
"pos"_ostr, u"0"_ustr);
}
We use loadAndSave to open the file since this is an export test, then we do some checks on the document model using assertXPath. To learn more about using assertXPath you can look in the README file located in
sw/qa/extras/README
Once the test is written and it fails with the bug fix reverted, you can switch back to a state where the bug is fixed and run the test again.
If it now passes, you’ve successfully written a test from the MissingUnitTest wiki 😃 🏆
You would now submit the test as a patch to Gerrit, I won’t go into the details on how to do that here. Follow the link below for more information.
https://wiki.documentfoundation.org/Development/gerrit
For more information about writing tests check, out the Document Foundation Wiki.
https://wiki.documentfoundation.org/Development/Cpp_Unit_Tests
Thanks for reading, I hope this was helpful. Writing a test is great way to learn how to get involved in the LibreOffice project. I’d encourage anyone thinking about contributing to checkout the links above or click the link below for some general information.
https://www.libreoffice.org/community/get-involved/
Written with StackEdit.