Skip to content

jsonclient: don't depend on error text from other packages in tests#1741

Merged
phbnf merged 1 commit intogoogle:masterfrom
neild:master
Nov 27, 2025
Merged

jsonclient: don't depend on error text from other packages in tests#1741
phbnf merged 1 commit intogoogle:masterfrom
neild:master

Conversation

@neild
Copy link
Copy Markdown
Contributor

@neild neild commented Nov 25, 2025

TestPostAndParse depends on the exact text of errors returned by encoding/json.
The text of some of these error changes when GOEXPERIMENT=jsonv2 is set,
causing the test to fail.

Change the test to not depend on the text of errors that are not contained
in this module.

@neild
jsonclient: don't depend on error text from other packages in tests
ebc0b63 
TestPostAndParse depends on the exact text of errors returned by encoding/json.
The text of some of these error changes when GOEXPERIMENT=jsonv2 is set,
causing the test to fail.

Change the test to not depend on the text of errors that are not contained
in this module.
@neild neild requested a review from a team as a code owner November 25, 2025 18:33
@neild neild requested review from phbnf and removed request for a team November 25, 2025 18:33
@roger2hk
Copy link
Copy Markdown
Contributor

roger2hk commented Nov 25, 2025

/gcbrun

1 similar comment
@phbnf
Copy link
Copy Markdown
Contributor

phbnf commented Nov 26, 2025

/gcbrun

@phbnf
Copy link
Copy Markdown
Contributor

phbnf commented Nov 26, 2025

These failures seem unrelated to this change. The first one was a timeout:

--- FAIL: TestAddChainRetries (23.68s)
    --- FAIL: TestAddChainRetries/multiple_retries (6.07s)
        logclient_test.go:474: #4 Submission took an unexpected length of time: 6.073546537s, expected ~5s
FAIL

The second one is a server startup failure:

Launching CT personalities
Starting CT HTTP server on localhost:6962, metrics on localhost:6963
/workspace/trillian/integration/ct_functions.sh: line 47: ./ct_server: No such file or directory

I can see some proto diffs in the tests though, @roger2hk , I think we need to regenerate protos after #1736? I thought that tests would fail if there was a diff, but it looks like tests in that PR didn't fail even though there was a diff?

@roger2hk
Copy link
Copy Markdown
Contributor

roger2hk commented Nov 26, 2025

These failures seem unrelated to this change. The first one was a timeout:

--- FAIL: TestAddChainRetries (23.68s)
    --- FAIL: TestAddChainRetries/multiple_retries (6.07s)
        logclient_test.go:474: #4 Submission took an unexpected length of time: 6.073546537s, expected ~5s
FAIL

The second one is a server startup failure:

Launching CT personalities
Starting CT HTTP server on localhost:6962, metrics on localhost:6963
/workspace/trillian/integration/ct_functions.sh: line 47: ./ct_server: No such file or directory

I can see some proto diffs in the tests though, @roger2hk , I think we need to regenerate protos after #1736? I thought that tests would fail if there was a diff, but it looks like tests in that PR didn't fail even though there was a diff?

AFAIK. The proto diff in the tests will not fail the check in this repository. The No such file or directory error is a known issue (#1529).

I've triggered to retry the failed test.

@phbnf phbnf merged commit 39ac73b into google:master Nov 27, 2025
7 checks passed
himaschal pushed a commit to digicert/certificate-transparency-go that referenced this pull request Jan 23, 2026
@neild @himaschal
jsonclient: don't depend on error text from other packages in tests (g…
f731f22 
…oogle#1741)

TestPostAndParse depends on the exact text of errors returned by encoding/json.
The text of some of these error changes when GOEXPERIMENT=jsonv2 is set,
causing the test to fail.

Change the test to not depend on the text of errors that are not contained
in this module.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phbnf phbnf phbnf approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@neild @roger2hk @phbnf