[Okular-devel] Review Request 126192: Fix mainshelltest (and as a side-effect docdata saving)

[Albert Astals Cid]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126192/#review88975
-----------------------------------------------------------



autotests/mainshelltest.cpp (line 294)
<https://git.reviewboard.kde.org/r/126192/#comment60903>

    Please leave the single dash, we need to support that since we've supported for 10 years, changing the test is not good.



autotests/mainshelltest.cpp (line 339)
<https://git.reviewboard.kde.org/r/126192/#comment60904>

    This doesn't look like a good thing, why is the exit status now not 0 if it was before. Doesn't seem like we should change the test but fix whatever makes the test fail, no?



autotests/mainshelltest.cpp (line 439)
<https://git.reviewboard.kde.org/r/126192/#comment60905>

    Leave the single dash.



core/document.cpp (line 1479)
<https://git.reviewboard.kde.org/r/126192/#comment60906>

    this change seems unrelated?



core/document.cpp (line 2248)
<https://git.reviewboard.kde.org/r/126192/#comment60907>

    Is anyone taking care of migrating the old files to the new location or are we just losing them all? Not that i want this fixed in this RR but wondering if anyone of the people involved in the frameworks port thought about that or not.



core/document.cpp (line 2256)
<https://git.reviewboard.kde.org/r/126192/#comment60908>

    Remove this variable since you're not using it?



shell/shell.h (line 72)
<https://git.reviewboard.kde.org/r/126192/#comment60909>

    I don't really think you need the comment here, the master branch is correctly using QString, i just think that whoever did the port did not do it correctly.



shell/shell.cpp (line 499)
<https://git.reviewboard.kde.org/r/126192/#comment60910>

    Why do we need this? I would sincerely prefer if the offending calls to setActiveTab are fixed instead of just workarounding them


- Albert Astals Cid


On nov. 30, 2015, 8:39 p.m., Alex Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126192/
> -----------------------------------------------------------
> 
> (Updated nov. 30, 2015, 8:39 p.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> ---
> 
> Try fixing mainshelltest: one more passes now
> 
> QUrl is not automatically exposed to DBus so the DBus call would fail.
> Changed the parameter to QString instead so that DBus invocation works.
> 
> ---
> 
> Fix crash due to out of bounds list access
> 
> Probably due to differences between KTabWidget and QTabWidget
> 
> ---
> 
> mainshelltest: set QStandardPaths to test mode
> 
> ---
> 
> Parse command line flags with a single dash as a long option
>     
> Unlike KCmdLineArgs QCommandLineParser treats options starting with a
> single minus as multiple short options by default.
>     
> Previously okular -unique would fail with the following error:
> Unknown options: u, n, i, q, u, e.
>     
> Also changed mainshelltest to use two dashes in case this behaviour
> should change in the future.
>     
> mainshelltest failures have been reduced from 15 to 4 by this commit.
> 
> ---
> 
> Fix docdata saving and added a warning message if it fails
> 
> If the ~/.local/okular/docdata directory didn't exist previously
> creating the docdata file would fail as there are missing paths.
> 
> It seems that KStandardDirs used to create the okular/docdata directory
> automatically, with QStandardPaths we have to create it manually.
> 
> mainshelltest is down to one failed test now
> 
> ---
> 
> Fix final test case in mainshelltest
> 
> as we call QProcess::terminate the exit code will not be 0
> 
> 
> Diffs
> -----
> 
>   autotests/mainshelltest.cpp 118cfbc1c18fdefad5f933d6de4b85820bfb7379 
>   core/document.cpp 04a29fd1be29f1c464b2d9d08ae20ba9e0b937ef 
>   shell/main.cpp 77847bacb5cdb426089626f63a0ba20c6b12bc9e 
>   shell/okular_main.cpp fdee69458baa0b111be9613e844834b7069fca00 
>   shell/shell.h c16a0b266008c3217f908aa92cc42103d62c5f4a 
>   shell/shell.cpp e69ecbb5e1bde36dbd00e5ff05cdad4e3d2a5f8d 
> 
> Diff: https://git.reviewboard.kde.org/r/126192/diff/
> 
> 
> Testing
> -------
> 
> mainshelltest now passes and documents are opened where I last viewed them
> 
> 
> Thanks,
> 
> Alex Richardson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20151130/490b3fc0/attachment-0001.html>