Skip to content
This repository was 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
szpak wants to merge 1 commit intonebula-plugins:gradle-1.12from
szpak-forks:topic/collection
Open

#3 - Base test case for support for collections and maps#7
szpak wants to merge 1 commit intonebula-plugins:gradle-1.12from
szpak-forks:topic/collection

Conversation

@szpak
Copy link
Copy Markdown
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
Copy link
Copy Markdown

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

@bmuschko
Copy link
Copy Markdown
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