planetfederal / qgis-geoserver-plugin

GNU General Public License v2.0
62 stars 52 forks source link

adding font marker geoserver sld compatibility #51

Closed ddespres closed 8 years ago

luipir commented 8 years ago

about the CI build fail, we are working to solve it. It does not seem related with this PR.

luipir commented 8 years ago

Hi, @ddespres

did you have a look to plugin tests? because your PR seems to solve a particular problem, would be possible to create a test basing on your use case and your sample data? in this way we can prevent regressions.

if you need help to create the test, here we are to support you.

ddespres commented 8 years ago

Hi Luigi,

No I didn’t looked at the test but I did it a few minutes ago and I don’t see where I can add a test case. I’ve not found a section in the test covering the “adaptation” of Qgis sld to Geoserver sld, but maybe I’ve missed something. If you can help me start,

Thanks

Damien

luipir commented 8 years ago

the entry point is test/testplugin.py where you can define

1) functional test (for the free available boundless tester plugin) that are tests where steps are defined to be executed in a semi authomatic way (more info in the tester plugin documentation) 2) add unit tests

unit tests are classic unit tests... probably that can be interesting for you are that in catalogtests, e.g. testVectorStylingUpload

ddespres commented 8 years ago

unit tests are not running on my PC (windows) so it's a little bit difficult. I've tried to add a Layer in existing Qgis test project and make it use a custom symbo but with my QGIS (2.16) but it changes a lot of things in the Qgs file when i save so i don't think it's a good way to process as it may break the other tests the projet is based upon.

luipir commented 8 years ago

HOW TO RUN ON WIN: A) install tester plugin https://github.com/boundlessgeo/qgis-tester-plugin B) install (or activate) geoserver plugin (in this way tests are injected in the tester plugin C) run tests against the geoserver test instance... IIRC you can configure the endpoint. I sugest you to avoid to run PKI tests because you need to configure certs on server side

HOW TO INTEGRATE TEST DATA If you are lost in the test organization, please share your test data (a subset) the expected rendering and the SLD to import... and would be great to have the procedure you follow to say "it work"... I'll try to help to create a test.

ddespres commented 8 years ago

Ok i've installed the plugin. To have it use my geoserver i've added boundless-test in my /etc/host otherwise it is not configurable. I will try to run some tomorrow when i have more time. As for my tests, there is no need of a particular dataset, Just a point layer where you choose to use a Font symbol for the marker type. you choose for example Family font "DejaVu Sans", in the character table you choose a character (character F for example which have value 0x72) and you publish. Then check in the sld that there is sld:WellKnownNamettf://DejaVu Sans#0x72/sld:WellKnownName

luipir commented 8 years ago

thank you for the detailed step sequence

luipir commented 8 years ago

great @ddespres :)

ddespres commented 8 years ago

yes, hard to get it working (my geoserver was too busy i think )but test plugin is interesting and now i'm more familiar with qgis api ;-)

ddespres commented 8 years ago

arff not working with CI, i check... Strange it worked when i launched the test individually with my QGIS

ddespres commented 8 years ago

i've recommit some files that was reverted. First time with github for me sorry. Now i think the PR is complete. But CI is failing do you know why?

luipir commented 8 years ago

@ddespres no problem :) now I'm checking if travis fails due to some our problems

luipir commented 8 years ago

@ddespres seems a problem instruduced in the test envirnment, not by your code. @elpaso did you changeed something in the CI config? build start to fail with an Alexandre's commit that does not involve almost any code modification.

ddespres commented 8 years ago

Hi all, When do you think you could merge this pull request with the master branch? Don't hesitate if you need anything else from me to complete the PR.

luipir commented 8 years ago

Hi @ddespres ok for me... any objection @volaya ?