Skip to content

Conversation

brunoerg
Copy link
Contributor

This PR adds test coverage to the endpoint /tx (rest) passing an invalid and an unknown txid to test its return.
Invalid -> should return status code 400 (bad request)
Unknown -> should return status code 404 (not found)

@fanquake fanquake added the Tests label Jan 13, 2022
@brunoerg brunoerg force-pushed the 2022-01-rest-functional branch from cc73f53 to a6376a8 Compare January 13, 2022 11:35
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK a6376a8

A couple of (more self-documenting?) ideas for fun, feel free to ignore.

        invalid_txid = "abc"
        resp = self.test_rest_request(uri=f"/tx/{invalid_txid}", ret_type=RetType.OBJ, status=400)
        assert_equal(resp.read().decode('utf-8').rstrip(), f"Invalid hash: {invalid_txid}")

        unknown_txid ="0000000000000000000000000000000000000000000000000000000000000000"
        resp = self.test_rest_request(uri=f"/tx/{unknown_txid}", ret_type=RetType.OBJ, status=404)
        assert_equal(resp.read().decode('utf-8').rstrip(), f"{unknown_txid} not found")
        invalid_txid = "abc"
        unknown_txid = "0000000000000000000000000000000000000000000000000000000000000000"
        for tx in [{'id': invalid_txid, 'status': 400, 'error': f"Invalid hash: {invalid_txid}"},
                   {'id': unknown_txid, 'status': 404, 'error': f"{unknown_txid} not found"}]:
            resp = self.test_rest_request(uri=f"/tx/{tx['id']}", status=tx['status'], ret_type=RetType.OBJ)
            assert_equal(resp.read().decode('utf-8').rstrip(), tx['error'])

@brunoerg brunoerg force-pushed the 2022-01-rest-functional branch from a6376a8 to 6e3caa9 Compare January 13, 2022 19:37
@brunoerg
Copy link
Contributor Author

force-pushed addressing @jonatack's comment. Instead of creating invalid_txid and unknown_txid I used invalid_param and unknown_param to use them in other scenarios (e.g invalid blocks) that use the same values.

@brunoerg brunoerg force-pushed the 2022-01-rest-functional branch from 6e3caa9 to bd52684 Compare January 13, 2022 20:02
@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 14, 2022

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24098 (rest: Use query parameters to control resource loading by stickies-v)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Copy link
Contributor

@kallewoof kallewoof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK bd52684

@maflcko maflcko merged commit 5c3bfee into bitcoin:master Jan 19, 2022
@jonatack
Copy link
Member

Posthumous ACK

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 20, 2022
bd52684 test: rest /tx with an invalid/unknown txid (brunoerg)

Pull request description:

  This PR adds test coverage to the endpoint `/tx` (rest) passing an invalid and an unknown txid to test its return.
  Invalid -> should return status code 400 (bad request)
  Unknown -> should return status code 404 (not found)

ACKs for top commit:
  kallewoof:
    ACK bd52684

Tree-SHA512: a7fbb63f30d06fc0855133a36e8317c7930ba13aa2b4a2dd1fc35079d59eacace72e1ffe7ae1b3e067066fe51792415940d72d923e83a659a0d5965e4110b32a
@bitcoin bitcoin locked and limited conversation to collaborators Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants