Related items data converter fix with tests: supports explicit value_…

[seanupton]

…type specified in field when using collections of UUID values. This is backward-compatible with previous conversion to field values.

Add-ons using collection of BytesLine values for UUIDs break without this fix, when using the mockup related items widget (which has strings submitted in JSON marshalled to unicode without regard to actual/declared field value type). This PR addresses that, and is backward compatible with existing default behavior (allowing use of TextLine/unicode or BytesLine/str to store UUID values).

[thet]

Why should the value_type be a instance of tuple?

[seanupton]

@thet You are correct, that was extraneous (only matters for collection types, non-sequitur here). Corrected in 1e462ca (also in PR).

[thet]

+1 for this. But before merging, I just want to know about the usecase - which datatype are you converting your uuid values to? the tests do not reflect that (maybe you can update them accordingly...).

plus: you might be interested in my refactoring of the related items widget (nearly finished. i just can't effort the time to fix the probably numerous CMFPlone robot tests failures before submitting it as final PR.)
plone/mockup#676

[seanupton]

@thet use case:

schema.List(value_type=schema.BytesLine(constraint=is_uuid))

The test now loops through three field scenarios reflects collection provided with:

• TextLine (only thing that could have worked because JSON strings treated as unicode).
• BytesLine (str makes more sense, IMHO, than TextLine)
• No value_type: default to unicode values, as this is what is backward compatible.

All my add-ons store list of str elements (declared as List of BytesLine) for content references; keeping it that way avoids migration. IIRC, there are other add-on authors who have done this, but I do not have specific example.

FWIW, I'm also working on backporting this into same converter in plone.app.widgets 1.x branch.

[seanupton]

Bump... anyone want to merge this?

The only thing Jenkins check was choking on was a false positive on a robot test unrelated to this commit (and said test runs fine locally, so this is likely selenium bug), see: https://community.plone.org/t/robot-test-failing-on-pr-issue-seems-indeterminate/2384

Cc: @thet

[seanupton]

Keep hitting more weird robot failures that are ONLY happening on Jenkins server, not in local testing. I don't see these failures in any way related to my change.

@jensens @thet any advice? I'm going to retry the Jenkins build again, assuming this is just some strange intermittent Jenkins problem, not actually a test problem.

This works locally (while running bin/robot-server against appropriate profile, selenium uses Firefox 46):

buildout.coredev sean$ ./bin/robot-debug src/plone.app.contenttypes/plone/app/contenttypes/tests/robot/test_collection_location_criterion.robot
==============================================================================
Test Collection Location Criterion
==============================================================================
Scenario: Test Relative Location Criterion                            | PASS |
------------------------------------------------------------------------------
Scenario: Test Absolute Location Criterion :: This sometimes fails... | PASS |
------------------------------------------------------------------------------
Test Collection Location Criterion                                    | PASS |
2 critical tests, 2 passed, 0 failed
2 tests total, 2 passed, 0 failed
==============================================================================
Output:  /Users/sean/projects/buildout.coredev/output.xml
Log:     /Users/sean/projects/buildout.coredev/log.html
Report:  /Users/sean/projects/buildout.coredev/report.html

[jensens]

I have no idea whats going on there. My opinion is to mark these tests as unstable. @tisto is against it and iirc said that fixing and debugging robot tests is the same as unittests, so this should be able to fix. But I failed at it, because I can get the error to pop up on my local machine and alo I have no access to jenkins on admin level inin order to try to debug there.

[seanupton]

Well, retrying seemed to do the trick.

I think this is ready to merge.

[thet]

sorry for not coming back earlier to this one. Missed the notification emails. I'm going to merge this one.