#1124 Reason explanation id ambiguity

[seregamorph]

The solution proposal for #1124

Sample failure message:

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:reason'.
> A failure occurred while executing com.autonomousapps.tasks.ReasonTask$ExplainDependencyAdviceAction
   > Could not create an instance of type com.autonomousapps.tasks.ReasonTask$ExplainDependencyAdviceAction.
      > Coordinates ':lis' matches more than 1 dependency [:list, :list-default, :list-impl]

and at the same time it works if --id is specified as :list as it fully matches one of the dependencies full name.

Sample test cases explaining updated behavior. Consider a project with these project dependencies:

• :list
• :list:sublist
• :list-default
• :list-impl

and library dependencies

• com.squareup.okio:okio-jvm:3.0.0
• com.squareup.okio:okio:3.0.0

--id	res	message
:li	❌	Coordinates ':li' matches more than 1 dependency [:list, :list:sublist, :list-default, :list-impl]
:list	✅	You asked about the dependency ':list'
:list:	✅	You asked about the dependency ':list:sublist'
:list-d	✅	You asked about the dependency ':list-default'
com.squareup.okio:oki	❌	Coordinates 'com.squareup.okio:oki' matches more than 1 dependency [com.squareup.okio:okio-jvm:3.0.0, com.squareup.okio:okio:3.0.0]
com.squareup.okio:okio	✅	You asked about the dependency 'com.squareup.okio:okio:3.0.0
com.squareup.okio:okio:	✅	You asked about the dependency 'com.squareup.okio:okio:3.0.0
com.squareup.okio:okio:3.0.0	✅	You asked about the dependency 'com.squareup.okio:okio:3.0.0
com.squareup.okio:okio-	✅	You asked about the dependency 'com.squareup.okio:okio-jvm:3.0.0

[autonomousapps]

Would you be willing to write a test that specifically exercises this new code?

[seregamorph]

For unit testing it looks trivial, but perhaps you mean another type of test. This is my first submission here, so I'm not quite familiar with test approach in this project - please give a good reference what you mean :)

[seregamorph]

Well, I see failed test and probably functionalTest jvm.BundleSpec can be a good candidate for enhancement?

[autonomousapps]

I think you could update JvmReasonSpec maybe. Most of the time, new tests can be written by copy/pasting other related tests and tweaking slightly.

[seregamorph]

@autonomousapps can you please recheck the PR?

[seregamorph]

I've discovered one more related problem. The project submodules are represented with both first and second coordinates key segment. Consider sample module with dependencies that have keys in dependencyUsages.entries:

• "demo-gradle-multi-module:list|:list"
• "demo-gradle-multi-module:list:sublist|:list:sublist"

and the filter --id=demo-.

Old behavior: it will take first non-ordered, there is ambiguity. In my experiment it took

----------------------------------------
You asked about the dependency 'demo-gradle-multi-module:list'.
There is no advice regarding this dependency.
----------------------------------------

The new behavior:

* What went wrong:
Execution failed for task ':app:reason'.
> A failure occurred while executing com.autonomousapps.tasks.ReasonTask$ExplainDependencyAdviceAction
   > Coordinates 'demo-' matches more than 1 dependency [:list, :list:sublist]

Instead of ambiguity it throws failure. But the error message can be confusing: we were filtering by demo-*, got :list*.
But from my perspective it's not a new problem, it does not make it worse than it was and I'd say it's a succession behavior :)

WDYT?

[seregamorph]

Do you still have concerns or just need more time for better validation of the change?

[autonomousapps]

Do you still have concerns or just need more time for better validation of the change?

Just more time. I've been underwater at work. Thanks for your patience.

[autonomousapps]

does this need to be internal instead of private?

[seregamorph]

because of accessing from the unit test

[autonomousapps]

Thank you for this great unit test