Add unit tests for Folder View
ClosedPublic

Authored by amantia on Oct 24 2017, 1:21 PM.

Details

Summary

Add some tests for FolderModel and Positioner classes.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
amantia created this revision.Oct 24 2017, 1:21 PM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 24 2017, 1:21 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript

Add in toplevel:
if(BUILD_TESTING)

add_definitions(-DBUILD_TESTING)

endif(BUILD_TESTING)
+
create a folderplugin_private_export.h as
#ifndef KSIEVEUIPRIVATE_EXPORT_H
#define KSIEVEUIPRIVATE_EXPORT_H

#include "ksieveui_export.h"

/* Classes which are exported only for unit tests */
#ifdef BUILD_TESTING

  1. ifndef FOLDERPLUGIN_TESTS_EXPORT
  2. define FOLDERPLUGIN_TESTS_EXPORT FOLDERPLUGIN_EXPORT
  3. endif

#else /* not compiling tests */

define FOLDERPLUGIN_TESTS_EXPORT

#endif

#endif

> we avoid to export symbol when we don't build test => distro doesn't generate lib with unnecessary exported symbol.

containments/desktop/plugins/folder/CMakeLists.txt
43

Use
if(BUILD_TESTING)

add_subdirectory(tests)

endif()

it's avoid to build tests when distro builds it.

missing copyright in new file too.

mlaurent requested changes to this revision.Oct 24 2017, 4:00 PM
This revision now requires changes to proceed.Oct 24 2017, 4:00 PM
amantia updated this revision to Diff 21281.Oct 25 2017, 6:54 AM

Implement changes requested by Laurent.

mlaurent added inline comments.Oct 25 2017, 7:59 AM
containments/desktop/plugins/folder/folderplugin_private_export.h
2

Missing copyright

containments/desktop/plugins/folder/tests/foldermodeltest.cpp
2

Missing copyright

88

QStringLiteral(...).arg

103

QStringLiteral(...).arg(...)

133

QStringLiteral("...) same for other line

containments/desktop/plugins/folder/tests/foldermodeltest.h
2

missing copyright

containments/desktop/plugins/folder/tests/positionertest.cpp
2

copyright

54

nullptr;

56

nullptr;

containments/desktop/plugins/folder/tests/positionertest.h
2

missing copyright

amantia updated this revision to Diff 21284.Oct 25 2017, 8:13 AM

Added copyright notice

hein accepted this revision.Oct 25 2017, 8:35 AM

Thanks for working on this! We really need better test coverage in Plasma bits.

I'm a little bit unhappy about littering headers with FOLDERPLUGIN_TESTS_EXPORT, but I guess it's the lesser evil of statically linking and building the code twice.

The tests itself look like a good start.

(Aside: If you have bigger plans for upcoming FV contributions, it'd be cool to sit down on IRC and talk a bit. I had a long conversation with Milian last month, I guess/hope he passed that info on.)

amantia updated this revision to Diff 21287.Oct 25 2017, 9:02 AM

Use QStringLiteral and nullptr

mlaurent accepted this revision.Oct 25 2017, 9:11 AM
This revision is now accepted and ready to land.Oct 25 2017, 9:11 AM
amantia updated this revision to Diff 21375.Oct 26 2017, 1:49 PM

Add back unintentionally removed generate_export_header

amantia updated this revision to Diff 21376.Oct 26 2017, 1:52 PM

Now add back the generate_export_header for real

mlaurent updated this revision to Diff 21377.Oct 26 2017, 2:28 PM
  • Add more autotest. Test default value. Test menu action
hein accepted this revision.Oct 27 2017, 7:11 AM

Please don't extend already-accepted review requests with new code without requesting more review. Otherwise the status stays "Accepted" on code that hasn't actually been reviewed, which is confusing and holds things up :)

New test looks fine, though.

This revision was automatically updated to reflect the committed changes.

I approved and actually suggested Laurent to update the diff. Sorry for the confusion.

It looks like it broke building with BUILD_TESTING=OFF:

/home/abuild/rpmbuild/BUILD/plasma-desktop-5.11.90git.20171028T043842~44827507/containments/desktop/plugins/folder/foldermodel.h:74:45: error: expected initializer before ':' token

Sorry about that, I will fix asap (probably in the evening).