Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[travis] Use Qt 5.7 #3400

Closed
wants to merge 4 commits into from
Closed

[travis] Use Qt 5.7 #3400

wants to merge 4 commits into from

Conversation

m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented Aug 16, 2016

No description provided.

@@ -21,5 +21,5 @@ export CCACHE_TEMPDIR=/tmp

DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

xvfb-run ctest -V -E "qgis_openstreetmaptest|qgis_wcsprovidertest|$(cat ${DIR}/blacklist.txt | paste -sd '|' -)" -S ./qgis-test-travis.ctest --output-on-failure
# xvfb-run ctest -V -E "qgis_openstreetmaptest|qgis_wcsprovidertest" -S ./qgis-test-travis.ctest --output-on-failure
# xvfb-run ctest -V -E "qgis_openstreetmaptest|qgis_wcsprovidertest|$(cat ${DIR}/blacklist.txt | paste -sd '|' -)" -S ./qgis-test-travis.ctest --output-on-failure
Copy link
Member

@3nids 3nids Aug 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be nicer to have blacklisted tests listed in a file rather than hard written in a command line?
i.e. keep the blakclist.txt file and move openstreetmap and wcs in the file

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do that once all others are ok to flag flaky tests (like these).
Until all tests are ok on Qt5 I'll occasionally run them all and copy paste the failing ones into blacklist.txt. In this case I don't want to take care of manually addiong osm and wcs tests.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speaking of flaky tests - I regularly see failures of the WFS tests on unrelated commits. Should these be blacklisted until the tests are made more robust?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, at least the failing bits. I realize myself that I start to ignore test results because of too many false alarms.
OgcUtils is also a candidate. CC @rouault

@3nids
Copy link
Member

3nids commented Sep 5, 2016

@m-kuhn any chance that @nirvn improved the situation here?

@m-kuhn
Copy link
Member Author

m-kuhn commented Oct 21, 2016

I wonder what's going on here. Travis gets a

undefined reference to `QSqlCachedResult::~QSqlCachedResult()'

https://dash.orfeo-toolbox.org/viewBuildError.php?buildid=248994

This destructor has been dropped between Qt 5.6 and Qt 5.7. What I don't know is where this could still be declared, everything was built from Qt 5.7 source using the osgeo4travis scripts, so no trace of Qt 5.6 should be left, ccache has been cleaned before the test to avoid leftovers from previous builds on the travis infrastructure...

If anyone has a pointer towards how to solve these issues, this would help greatly to finally get a test infrastructure with fixed rounding issues (and a recent gdal RC).

@rouault, @nyalldawson, @wonder-sk

@nyalldawson
Copy link
Collaborator

I've struggled with building QGIS on 5.7 too. On Ubuntu I got a bunch of very cryptic errors within the qt headers themselves, which I couldn't find any mention of on any search. I know Nathan struggled with a 5.7 build (windows) too. I wonder if there's some remnant of the old qt4 stuff still hanging around in the cmake files somewhere which could be contributing to this... It's the only thing which jumps to mind.

@m-kuhn
Copy link
Member Author

m-kuhn commented Oct 25, 2016

It worked for Android using the sdk downloaded directly from Qt with this cmake config.

@m-kuhn m-kuhn closed this Nov 15, 2016
@m-kuhn m-kuhn reopened this Nov 15, 2016
@m-kuhn
Copy link
Member Author

m-kuhn commented Nov 15, 2016

Finally working!

Thanks to e9c4090 and a small missing fix for the pyqt resource generator!

But, we've got a small problem with some tests: seems all our rendered images are one pixel higher than before...
And some other minor rounding issues.

https://dash.orfeo-toolbox.org/viewTest.php?onlyfailed&buildid=251862

@3nids
Copy link
Member

3nids commented Nov 16, 2016

Whenever, we get this merged, shall we have a Qt 5.2 running on Travis as this is our requirement?
That might avoid merging incompatible stuff that could be problematic to revert when we realize it?

@nyalldawson
Copy link
Collaborator

Whenever, we get this merged, shall we have a Qt 5.2 running on Travis as this is our requirement?

Not sure, but I've been thinking we could bump the min to 5.3 since trusty is now unsupported (lack of Qca/QSci stuff)

@nyalldawson
Copy link
Collaborator

But, we've got a small problem with some tests: seems all our rendered images are one pixel higher than before..

Hmmm... it's not just that. Check out https://dash.orfeo-toolbox.org/testDetails.php?test=48632817&build=251862 or https://dash.orfeo-toolbox.org/testDetails.php?test=48632837&build=251862 . Seems like something's change in how images are initialised...

@nyalldawson
Copy link
Collaborator

This is very odd - the images are definitely initialised, eg https://github.com/qgis/QGIS/blob/master/tests/src/core/testqgspainteffect.cpp#L336

@nyalldawson
Copy link
Collaborator

Looking into this, I suspect the problem is QgsRenderChecker::drawBackground. That could account for the apparent 1 pixel shift in composition tests too.

@m-kuhn m-kuhn mentioned this pull request Nov 21, 2016
@m-kuhn
Copy link
Member Author

m-kuhn commented Nov 22, 2016

The tests qgis_painteffecttest and qgis_imageoperationtest pass with Qt 5.7 here locally (Fedora 25)

m-kuhn pushed a commit to m-kuhn/QGIS that referenced this pull request Apr 12, 2017
fix qgis#3400
also remove duplicate setting of background color (it is set by ProjectProperties)

(cherry-picked from 07db984)
@m-kuhn m-kuhn closed this Aug 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants