Add method finishedTraces to MockTracer.

[qudongfang]

Resolves #277

[sjoerdtalsma]

No other comments, thanks for the change

[qudongfang]

cc @pavolloffay

[whiskeysierra]

Alternatively it could be made read-only/unmodifiable using `Collections.unmodifiable*`. Has to be done deep though, i.e. to all contained maps as well.

…

On Mon, Aug 12, 2019, 11:53 Pavol Loffay ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java <#359 (comment)> : > @@ -95,6 +93,28 @@ public synchronized void reset() { return new ArrayList<>(this.finishedSpans); } + /** + * @return a copy of all finish()ed Traces(Spans) started by this MockTracer grouped by traceId and spanId in HashMap format. + */ + public Map<String, Map<String, Span>> finishedTraces() { It should return a MockSpan not span ------------------------------ In opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java <#359 (comment)> : > @@ -95,6 +93,28 @@ public synchronized void reset() { return new ArrayList<>(this.finishedSpans); } + /** + * @return a copy of all finish()ed Traces(Spans) started by this MockTracer grouped by traceId and spanId in HashMap format. It does not return a copy — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#359?email_source=notifications&email_token=AADI7HN6PIL7O3VMZYL4UULQEEXLDA5CNFSM4IK6KQ32YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBHME4A#pullrequestreview-273597040>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADI7HI6HEADKEMB3C2URJDQEEXLDANCNFSM4IK6KQ3Q> .

[qudongfang]

Alternatively it could be made read-only/unmodifiable using Collections.unmodifiable*

Thanks for your suggestion. I am not sure if we really need it to be a copy or read-only/unmodifiable.

[pavolloffay]

+1 on unmodifiable

[sjoerdtalsma]

Not sure on unmodifiable. Each call returns a new Map structure anyway.
Any pressing reasons for the unmodifiable, seeing that the map values must become unmodifiable as well?

[carlosalberto]

Agreed, I don't think we have to add the unmodifiable part. For example, finishedSpans() is defined as:

    public synchronized List<MockSpan> finishedSpans() {
        return new ArrayList<>(this.finishedSpans);
    }

[qudongfang]

+1 on unmodifiable

Done.

[whiskeysierra]

I wasn't aware that the one right returns a copy/snapshot. I'd keep consistency and do the same then.

…

On Wed, Aug 14, 2019, 12:14 qudongfang ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In opentracing-mock/src/main/java/io/opentracing/mock/MockTracer.java <#359 (comment)> : > * MockTracer.reset()). * * @see MockTracer#reset() */ public synchronized List<MockSpan> finishedSpans() { - return new ArrayList<>(this.finishedSpans); + return Collections.unmodifiableList(new ArrayList<>(this.finishedSpans)); Yes, It is a breaking change. I think it is not right to make finishedTraces unmodifiable while finishedSpans modifiable. @pavolloffay <https://github.com/pavolloffay> Shall we make them all modifiable? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#359?email_source=notifications&email_token=AADI7HITHDWM2SIMOEPELFDQEPLGVA5CNFSM4IK6KQ32YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBQPYXA#discussion_r313801693>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADI7HMQFW4IRMBA4TX7UKDQEPLGVANCNFSM4IK6KQ3Q> .

[coveralls]

Coverage increased (+0.6%) to 76.586% when pulling cf6bcce on qudongfang:finishedTraces into 70555ed on opentracing:master.

[carlosalberto]

Hey, thanks a lot for the updates, appreciated. It looks good now, and just left a small comment about keeping the comment regarding the returned MockSpans being a copy. After that we should be good to merge ;)

[carlosalberto]

Merged, thanks!