Skip to content

Commit 89d4998

Browse files
committedSep 28, 2016
[Server 3.0] Resurrect PyQgsServerWFST
1 parent 49e64ab commit 89d4998

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed
 

‎ci/travis/linux/script.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,5 @@ export CCACHE_TEMPDIR=/tmp
2121

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

24-
xvfb-run ctest -V -E "qgis_openstreetmaptest|qgis_wcsprovidertest|PyQgsOfflineEditingWFS|PyQgsWFSProviderGUI|qgis_ziplayertest|$(cat ${DIR}/blacklist.txt | paste -sd '|' -)" -S ./qgis-test-travis.ctest --output-on-failure
24+
xvfb-run ctest -V -E "qgis_openstreetmaptest|qgis_wcsprovidertest|PyQgsWFSProviderGUI|qgis_ziplayertest|$(cat ${DIR}/blacklist.txt | paste -sd '|' -)" -S ./qgis-test-travis.ctest --output-on-failure
2525
# xvfb-run ctest -V -E "qgis_openstreetmaptest|qgis_wcsprovidertest" -S ./qgis-test-travis.ctest --output-on-failure

9 commit comments

Comments
 (9)

m-kuhn commented on Sep 28, 2016

@m-kuhn
Member

FYI, the test was disabled because it seemed to produce unreliable results: 6214dc1

If that's fixed, that's good news :)

elpaso commented on Sep 28, 2016

@elpaso
ContributorAuthor

Oh, sorry, I thought it was removed when the switch to qt5/Py3 was made.

But generally speaking, I believe that we should think twice before removing a test and if it is really necessary, it should be done with a bit more respect for the test author and maybe saying it loud on the mailing list.

You must know very well that writing tests takes time and it's not the most exciting activity, the rm -rf (or blacklist either) approach in this case is not the best way to proceed in order to motivate volunteers to write and maintain tests.

In general, @dakcarto also agreed that we should remove the older server scripts that is much more complex to maintain in favour of the new one which is completely python based.

If the new server script is not reliable maybe we can find a way to improve its reliability, I wasn't aware of any problem with that.

m-kuhn commented on Sep 28, 2016

@m-kuhn
Member

We should probably have a method to blacklist tests in a different way.

When blacklisting them, they should still be executed but their success/failure noted somewhere and output logged. This way they wouldn't give false alarms on travis ci (which sometimes affect credibility of test results) but still allow for investigating the real source of troubles.

nyalldawson commented on Sep 28, 2016

@nyalldawson
Collaborator

Yes, this was blacklisted from Travis (not removed from QGIS!) a while back. Situation was that a bunch of these tests were regularly but randomly failing and we'd see many PRs fail because of this. It was causing frustration with new contributors who were struggling to understand why their work was causing issues. It's a tough call but if Travis can't be relied upon to only flag valid errors then people start ignoring it and pushing broken code.

Re announcing it publicly - that's what the commit message was for ;) but I guess not everyone tracks master close enough to catch every commit, so in retrospect a dev list announcement should have been made.

I'll ping you next time this fails on Travis (if it's not been resolved by other changes in the meantime) for you to look into.

nyalldawson commented on Sep 28, 2016

@nyalldawson
Collaborator

Re server scripts - happy for you to make the call, but standardising on one would be much appreciated! Can we move it from port 8081 to something less commonly used, or better yet to have it auto select a free port?

elpaso commented on Sep 28, 2016

@elpaso
ContributorAuthor

@nyalldawson yes, you can change the port by setting an env var: https://github.com/qgis/QGIS/blob/master/tests/src/python/qgis_wrapped_server.py#L31
Do you think that the problem is caused from the different concurrent builds? If that is the case, maybe there is something in the Travis env that we can use to avoid port clashing (for example a build counter and we could make port 6666 + $BUILD_COUNTER or something similar).

elpaso commented on Sep 28, 2016

@elpaso
ContributorAuthor

@nyalldawson , I'm testing this:

try:
    QGIS_SERVER_WFST_DEFAULT_PORT = os.environ['QGIS_SERVER_WFST_DEFAULT_PORT']
except:
    import socket
    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    s.bind(("",0))
    QGIS_SERVER_WFST_DEFAULT_PORT = s.getsockname()[1]
    s.close()

so far, so good.

nyalldawson commented on Sep 29, 2016

@nyalldawson
Collaborator

@elpaso looks good to me!

elpaso commented on Sep 29, 2016

@elpaso
ContributorAuthor

implemented in #3540

Please sign in to comment.