modelcontextprotocol / modelcontextprotocol/java-sdk
Bad assertion in testCallTool
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 3.7k
- Forks
- 1.1k
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 9
Description
In AbstractMcpSyncClientTests, there is a test case called {testCallTool](https://github.com/modelcontextprotocol/java-sdk/blob/110a8d1940ca2edf45524332dcc84b606e52d879/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java#L238)
on line 247 is this assertion:
assertThat(result.isError()).isNull();
Although currently satisfied by the 'everything server' (assertion passes with javascript server) this assumes that the result.isError() (a Boolean value) is null if there is no error. Since 'isError() is set to 'false' and not null on success (by java server at least) this line should probably be:
assertThat(result.isError()).isFalse();
and that the everything server should be fixed to return isError=false rather than isError=null as it does currently.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java at testCallTool and its assertion around line 247. Check the everything server's success response as well. Done means successful results assert isError() is false and the everything server returns false rather than null.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, javascript
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 75/100