Add return_conflicts
options to check_conflicts
function and enhance tests
#5000
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is motivated by the easyconfig tests where the result is hard to interpret:
First the wording: It sounds like the error is that no conflicts are detected (similar in other places)
Second the actual conflicts are printed to stderr somewhere hidden above with other test output
The new parameter allows returning the text instead of printing it.
The PR where I've seen this was PyTorch: It has a build dependency on something which happens to be a runtime dependency of a dependency. So I added a test that this is detected but not if it is a build dependency for both where there would be no issue.
With that I ran into our somewhat strange dictionary duplicating a couple values that are easyconfig parameters, properties or functions:
{ 'ec': ec, 'spec': ec.path, 'short_mod_name': ec.short_mod_name, ...}
As the robot-resolver uses that dict instead of an
EasyConfig
instance and I needed that in the test I factored out the creation of this dict into a new function. It is still strange... We should consider removing it or at least transforming it to a dataclass once we support only Python 3.7 to make it easier to reason about.Should we report only primary dependents in the error message? See this confusing message:
I.e. it collects transitive dependents while collecting transitive dependencies in https://github.com/Flamefire/easybuild-framework/blob/9cd02c9ea03c5bdf278f0760453306e795737a5f/easybuild/tools/robot.py#L178-L181