Update duplicate output error msg

[dberenbaum]

Fixes #8986

Before this PR:

$ dvc stage add -f -n train -d train.py -o model python train.py
ERROR: output 'model' is already specified in stages:
        - model.dvc
        - train

After this PR:

$ dvc stage add -f -n train -d train.py -o model python train.py
ERROR: output 'model' is already specified in stage: 'model.dvc'.
Use `dvc remove model.dvc` to stop tracking the overlapping output.

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (40484e8) 92.86% compared to head (8ef7c62) 92.86%.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #9138    +/-   ##
========================================
  Coverage   92.86%   92.86%            
========================================
  Files         458      458            
  Lines       37013    37028    +15     
  Branches     4355     5350   +995     
========================================
+ Hits        34373    34387    +14     
  Misses       2114     2114            
- Partials      526      527     +1     

Impacted Files	Coverage Δ
dvc/exceptions.py	95.83% <100.00%> (+0.14%)	⬆️
dvc/repo/stage.py	94.35% <100.00%> (+0.13%)	⬆️
tests/func/test_stage.py	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[omesser]

Can be compacted a bit more maybe if we tradeoff some "accuracy", e.g.

• dvc remove message can be unified
• maybe the multiline '\t-' variant is not needed

But is fine either way imo, approving

[daavoo]

Would be nice to add a test for this

[dberenbaum]

Any suggestions for where to add it?

[daavoo]

Probably in tests/fun/test_add, something like: https://github.com/iterative/dvc/blob/121d56b7391f5325e5eb6ff41448980f0ede00df/tests/func/test_add.py#L749-L753 or even just update that one and use match with the expected exception message

[daavoo]

Does the message vary if the order is different from the one in the test (i.e. you run dvc add first)?

[dberenbaum]

Yes it does. This PR doesn't change the error for dvc add, only for dvc stage add, so not sure if it makes sense to put it there. Had trouble finding any tests about dvc stage add beyond testing the cmd args.

[daavoo]

Ok. I understand.

From a quick look, I think I would personally add it to tests/func/test_stage.py then. Like:

def test_stage_add_duplicated_output(tmp_dir, dvc):
    tmp_dir.dvc_gen("foo", "foo") 
    dvc.add("foo")

    with pytest.raises(
        OutputDuplicationError,
        match="Use `dvc remove foo.dvc` to stop tracking the overlapping output."
    ):
        dvc.stage.add(name="duplicated", cmd="echo bar > foo", outs=["foo"])

But I am also confused about where the logic for StageLoad.create is/should be tested cc @skshetry

[dberenbaum]

Added the test you suggested @daavoo. It makes sense to me.

[dberenbaum]

• dvc remove message can be unified

Thanks @omesser. Tried to clean this up a bit.