Add support for struts 2.3 with tests #244
Conversation
shashank11p
changed the title
...rc/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/struts/Struts2Action.java
Outdated
Show resolved
Hide resolved
| public static String sample = "{'balance':1000.21,'is_vip':true,'num':100,'name':'foo'}"; | ||
|
|
||
| public String body() throws IOException { | ||
| jsonString = sample; |
There was a problem hiding this comment.
This is just a simple logic that whenever this api is called, it assigns jsonString a value(which in our case is same every time for tests), and that jsonString is returned as response. It is defined in struts.xml to take jsonString as result.
There was a problem hiding this comment.
It's more idiomatic to return the "sample" directly. Why do we need two properties jsonString and sample?
|
|
||
| public static String sample = "{'balance':1000.21,'is_vip':true,'num':100,'name':'foo'}"; | ||
|
|
||
| public String body() throws IOException { |
There was a problem hiding this comment.
Add @Override if the class overrides any method from the parent
There was a problem hiding this comment.
It does not override any method. This is our defined method which is an endpoint for /body calls.
.../io/opentelemetry/javaagent/instrumentation/hypertrace/struts/StrutsInstrumentationTest.java
Outdated
Show resolved
Hide resolved
.../io/opentelemetry/javaagent/instrumentation/hypertrace/struts/StrutsInstrumentationTest.java
Outdated
Show resolved
Hide resolved
|
@shashank11p in the first comment on the PR add a link to github issue that this PR is fixing e.g. #218. If you prefix it with resolves, fixes it will automatically close the issue once the PR is merged. Could you please also list the struts in the readme? |
pavolloffay
changed the title
resolves #218