Differential D9828

Create test app to test qml files
ClosedPublic
Actions

Authored by velurimithun on Jan 11 2018, 6:07 PM.

Details

Reviewers
mlaurent
Group Reviewers
Ruqola
Commits
R865:53b37cdb36d0: Create test app to test qml files
Summary

comboBox, QuickView

Test Plan

Create a widget and add combobox, label, button as user interface and use QQuickView to load qml files in seperate window

Diff Detail

Repository
R865 Ruqola
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes
mlaurent added inline comments.Jan 12 2018, 5:52 AM
tests/qmltest.cpp
34

why ?

66

use a identifier in combobox api => qvariant. It's safe as compare with qstring

> addItem(..., QVariant)

This revision now requires changes to proceed.Jan 12 2018, 5:52 AM
mlaurent added a comment.Jan 12 2018, 5:55 AM

now your error that you see when you try to load Desktop.qml normal as you didn't use RuqolaRegisterEngine
So you need to link with libruqolacore, initialize it etc.
And it will good if combobox is initialize with all qml file.

> you need to find a method for load them from current directory, so it avoids that I need to open it directly.

But we need to keep 'Add file' .

Regards

velurimithun updated this revision to Diff 25295.Jan 14 2018, 7:59 AM
velurimithun marked 14 inline comments as done.

Modify the code according to commemts

tests/qmltest.cpp
18

Sry, I forgot to change it to QStringLiteral after taking it from Qt help doc

34

I think here we are creating pointers and memory allocated for that will not be deleted by default destructor and it should be done by an user defined destructor.

to avoid MEMORY LEAK

66

Do we have any idea of what type of Identifier is to be used to identify files in (QVariant argument)

mlaurent requested changes to this revision.Jan 14 2018, 9:08 AM

I think that it still not work with current qml as you didn't use ruqolaengine...

tests/qmltest.cpp
66

A enum or other. As you want, just a specific value.
or boolean etc. but compare against a qstring is not safe if we change it by error in future.

tests/qmltest.h
46

????? why in header ???
I just told you that we need to initialize to nullptr for be sure that all is initialize to nullptr for avoiding that we forgot it.

This revision now requires changes to proceed.Jan 14 2018, 9:08 AM
velurimithun updated this revision to Diff 25316.Jan 14 2018, 2:06 PM
velurimithun marked an inline comment as done.

Initialize ruqolaRegisterEngine

velurimithun added a comment.Jan 14 2018, 2:10 PM

I'm getting following errors:

Starting /home/veluri/ruqola/build/tests/qmlFileTest...
qrc:/TakeVideoMessageDialog.qml:45:42: Unable to assign [undefined] to int
The program has unexpectedly finished.
The process was ended forcefully.
/home/veluri/ruqola/build/tests/qmlFileTest crashed.

After commenting that line 45 line in TakeVideoMessageDialog.qml also I'm getting error
Starting /home/veluri/ruqola/build/tests/qmlFileTest...
The program has unexpectedly finished.
The process was ended forcefully.
/home/veluri/ruqola/build/tests/qmlFileTest crashed.

velurimithun updated this revision to Diff 25330.Jan 14 2018, 5:33 PM

Check the file whether it is present in qrc:/ directory

if not, again take input from user.

mlaurent added inline comments.Jan 14 2018, 5:50 PM
tests/qmltest.cpp
30

where are the signal/slot for combobox and pushbutton ?

32

add parent

35

You forgot to delete it in destructor

61

Else ?
perhaps you can add a warning when we can't initialize it.

70

I want the list no if file is in qrc:/ otherwise I need to know the name.

Perhaps you can create liste from file in source dir directly.

velurimithun updated this revision to Diff 25404.Jan 15 2018, 4:38 PM
velurimithun marked 3 inline comments as done.

Write else case if engine not initialized

tests/qmltest.cpp
30

Signal and slot for pushButton are connected.

But, why signal for combobox?

70

Why do we need a list?

Now, it will check for input file in qrc:/ repository and if entered filename is not there in repo it again prompts the user to give filename.

velurimithun added a comment.Jan 15 2018, 4:40 PM

I'm getting the following error when I try to log the warning in RUQOLA_LOG although it is defined in ruqola_debug.h

/home/veluri/ruqola/tests/qmltest.cpp:64: error: undefined reference to `RUQOLA_LOG()'

velurimithun added a comment.Jan 15 2018, 4:48 PM

In cmakeLists.txt also I think there is no need to target_link_libreries link QT5::widget

Since all those are linked in libruqolacore :)

mlaurent added inline comments.Jan 15 2018, 5:43 PM
tests/qmltest.cpp
30

Ah you have a button for loading it... indeed not necessary to have a slot for it.

70

Because I don't want to know all qml filename :)
I have 30 qml file at the moment, so I don't want to know each name.
I want to have a list of all qml so I just need to select it, and it's easy to test them.

it's great to add new qml filename but for existing files I don't want to know each name.

velurimithun updated this revision to Diff 25437.Jan 16 2018, 5:49 AM
  • Add all qml files present in current dir
velurimithun marked an inline comment as done.Jan 16 2018, 5:52 AM
velurimithun added inline comments.
tests/CMakeLists.txt
79

here I think there is no need to link Qt5::Widgets, Qt5::Quick since those 2 were already linked in libruqolacore

velurimithun added a comment.Jan 16 2018, 5:55 AM

Please kindly check, our ruqolaRegisterEngine is not initializing , were are getting errors :/ already mentioned those in prev. comments

velurimithun updated this revision to Diff 25438.Jan 16 2018, 5:57 AM
  • change dir to ":/" from "qrc:/" in add qml file
mlaurent requested changes to this revision.Jan 16 2018, 6:53 AM
mlaurent added inline comments.
tests/CMakeLists.txt
76

lowercase for test apps thanks.

tests/qmltest.cpp
37

remove space after ":"

61

remove extra (...)

72

now we can see qml file it's good,
Could you use
combobox->addItem( <filename without :/>, filename);
and when you add "ombobox->addItem(QStringLiteral("Add file"));" use ombobox->addItem(QStringLiteral("Add file"), QString());

85

We can't add an external file ?
I think as you load all qml file from qrc it's not necessary to check here. We need to be able to load external qml file

89

when you cancel it it doesn't cancel.
But perhaps you can use qfiledialog no ?

108

use combobox->currentData() after changes

tests/qmltest.h
2

it's your copyright no ?

This revision now requires changes to proceed.Jan 16 2018, 6:53 AM
velurimithun updated this revision to Diff 25498.Jan 16 2018, 6:53 PM
velurimithun marked 7 inline comments as done.

Modify the code for dialogBox

tests/qmltest.cpp
61

This is not the extra method it is used in loadCombobox method.

velurimithun updated this revision to Diff 25499.Jan 16 2018, 7:00 PM
  • GIT_SILENT
mlaurent added inline comments.Jan 16 2018, 8:50 PM
tests/qmltest.cpp
61

I asked to remove '(' and ')' :)

velurimithun updated this revision to Diff 25519.Jan 17 2018, 5:52 AM
  • Remove extra code
velurimithun marked an inline comment as done.Jan 17 2018, 5:52 AM
mlaurent requested changes to this revision.Jan 17 2018, 7:17 AM
mlaurent added inline comments.
tests/qmltest.cpp
3

Remove it

24

Useful ?

76

"combobox->addItem(filename, QString());"
you need to convert as "combobox->addItem( <filename without :/>, filename);" as I wrote previously

78

add space after }

88

QFileDialog as requested please

108

already not done

tests/qmltest.h
3

Remove it

48

const'ref

This revision now requires changes to proceed.Jan 17 2018, 7:17 AM
velurimithun updated this revision to Diff 25526.Jan 17 2018, 9:18 AM
velurimithun marked 3 inline comments as done.
  • Use QFileDialog
velurimithun marked an inline comment as done.Jan 17 2018, 9:20 AM
velurimithun added inline comments.
tests/qmltest.cpp
24

for RUQOLA_LOG

61

:))
sry, I didn't understand!!

88

Yes, QFileDialog is more useful here than the prv. one!!

Thanks:)

mlaurent added inline comments.Jan 17 2018, 10:51 AM
tests/qmltest.cpp
24

ok but line with RUQOLA_LOG is commented no ?

velurimithun updated this revision to Diff 25573.Jan 18 2018, 6:25 AM
velurimithun marked an inline comment as done.
  • GIT_SILENT
mlaurent added inline comments.Jan 18 2018, 7:50 AM
tests/qmltest.cpp
85

const QString

106

isEmpty() is better.

108

} else {

mlaurent added a comment.Jan 18 2018, 7:50 AM

After theses last fixes it will be good :)

velurimithun updated this revision to Diff 25585.Jan 18 2018, 10:25 AM
velurimithun marked 3 inline comments as done.
  • use isEmpty() instead QString()
mlaurent accepted this revision.Jan 18 2018, 3:14 PM

Thanks :)

This revision is now accepted and ready to land.Jan 18 2018, 3:14 PM
Closed by commit R865:53b37cdb36d0: Create test app to test qml files (authored by velurimithun). · Explain WhyJan 18 2018, 4:15 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mtests/CMakeLists.txt (10 lines)
AMtests/qmltest.h (55 lines)
AMtests/qmltest.cpp (122 lines)

Diff 25600

View Options

tests/CMakeLists.txt

Loading...
View Options

tests/qmltest.h

Loading...
View Options

tests/qmltest.cpp

Loading...