Skip to content
This repository has been archived by the owner on Nov 27, 2023. It is now read-only.

#3 - Base test case for support for collections and maps #7

Open
wants to merge 1 commit into
base: gradle-1.12
Choose a base branch
from

Conversation

szpak
Copy link
Contributor

@szpak szpak commented Oct 27, 2014

Base test cases for #3 - Ability to override List/Set/Map property.

As there are many cases functional testing all of them with nebula-test plugin would be an overkill. I left 2 simple cases and moved other to unit/integration tests. The implementation would probably require to extract a mechanism from DotNotationWalkerOverrideStrategy to determine a field type (also a generic type for collections - like foo.class.declaredFields[0].getGenericType()) and leave only base types for ApacheCommonsTypeConverter. Therefore I left test in a very generic form.

I am not also sure about the name. ComplexType can suggest that complex/custom type can be passed. Collections and Maps is an alternative.

Probably not all test cases should be supported - to make the implementation easier. Also there are for sure cases I missed.

Regarding def as extension is exposed API I would consider the lack of support for fields defined as def or use specific default behavior which to cover only base cases (the easier way).

If you decide to keep so many test cases the duplication in where: part could be eliminated with some Groovy tricks on propertyName column.

@cloudbees-pull-request-builder

Nebula » gradle-override-plugin-pull-requests #5 SUCCESS
This pull request looks good

@bmuschko
Copy link
Contributor

Thanks for the pull request. I think I want to start with the simple cases and then build up complexity. As you mentioned we might want to extend the functionality of DotNotationWalkerOverrideStrategy. I'd probably start writing tests on that level.

The pull request has good ideas but I don't think I will pull it in as is. I am going to start with an implementation this or next week.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants