vlsi commented on PR #693:
URL: https://github.com/apache/jmeter/pull/693#issuecomment-1528952597
I rebased "option 2", and now I'm puzzled.
It fails on test cases like `assertEquals(new Header("accept", "*/*"),
httpSampler.getHeaderManager().getHeader(0));`
https://github.com/apache/jmeter/blob/5020590c2dd6564e87cc2295e0090d2c6526c436/src/protocol/http/src/test/java/org/apache/jmeter/gui/action/ParseCurlCommandActionTest.java#L254-L255
The code that compares elements like `if (file.equals(item)) {` seems to be
harder to spot.
Then, it is not that often that test elements are placed in sets. Well,
JMeter itself does that, however, it is unlikely plugins will deal with
`Map<TestElement`.
Then, we won't be able to make `TestElement#equal` final as it might break
compatibility with some of the plugins that happened to override the method for
their own reasons.
So it might be a safer choice to go for "option 1", and make
`TestElement#hashCode` and `TestElement#equals` compare element contents. Then
all the places that need element identity (e.g. `SearchByClass` that needs to
report individual elements even if their contents is the same) should use
`IdentityHashMap` or something like that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
