Skip to content

CASSGO-59 CASSGO-56 Test fix for hostpool package and newHostInfo() expose #1864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 27, 2025
Merged

CASSGO-59 CASSGO-56 Test fix for hostpool package and newHostInfo() expose #1864

merged 1 commit into from
Mar 27, 2025

Conversation

@tengu-alt
Copy link
Contributor

@tengu-alt tengu-alt commented Feb 21, 2025 •
edited
Loading

hostpool package test was using recently removed SetConnectAddress()
The test was refactored to create hostInfo by constructor func, and the newHostInfo constructor was exposed.
Also fix for the CASSGO-56

@tengu-alt tengu-alt changed the title CASSGO 59 Test fix for hostpool package and newHostInfoExpose CASSGO 59 Test fix for hostpool package and newHostInfo() expose Feb 21, 2025
@joao-r-reis joao-r-reis changed the title CASSGO 59 Test fix for hostpool package and newHostInfo() expose CASSGO-59 Test fix for hostpool package and newHostInfo() expose Feb 21, 2025
@joao-r-reis
Copy link
Contributor

joao-r-reis commented Feb 27, 2025

We should fix the CI issue here on this PR too so we know we're fixing the issue and there won't be regressions

@tengu-alt
Copy link
Contributor Author

tengu-alt commented Feb 27, 2025

We should fix the CI issue here on this PR

Understood, I'll do it.

@joao-r-reis
Copy link
Contributor

joao-r-reis commented Mar 18, 2025

@tengu-alt I don't see TestHostPolicy_HostPool in the CI log still, I think the fix is to add ./.. to go test, we should do it for all steps in the workflow I think

@tengu-alt tengu-alt force-pushed the cassgo-45-fix branch 2 times, most recently from a323a8f to 1da65ed Compare March 19, 2025 12:07
@tengu-alt
Copy link
Contributor Author

tengu-alt commented Mar 19, 2025

I think the fix is to add ./.. to go test, we should do it for all steps in the workflow I think

Agree, I've added the ./... to jobs go test, also I've added the unit build tags for other packages tests(e.g. murmur and lru) to prevent double run during integration tests.

@tengu-alt
Test fix for hostpool package
5931c12 
hostpool package test was refactored to create HostInfo via exported NewhostInfo() constructor.
NewHostInfo() was exposed.

patch by Oleksandr Luzhniy; reviewed by João Reis, for CASSGO-59
@tengu-alt
Copy link
Contributor Author

tengu-alt commented Mar 20, 2025

I had to remove the ./... from integration test runs because we already have those within env.args

@joao-r-reis joao-r-reis mentioned this pull request Mar 25, 2025
Merged
@tengu-alt
Copy link
Contributor Author

tengu-alt commented Mar 27, 2025

@joao-r-reis It seems that TestHostPolicy_HostPool is passed test. Please take a look.

@joao-r-reis joao-r-reis changed the title CASSGO-59 Test fix for hostpool package and newHostInfo() expose CASSGO-59 CASSGO-56 Test fix for hostpool package and newHostInfo() expose Mar 27, 2025
@joao-r-reis joao-r-reis merged commit c75ff5f into apache:trunk Mar 27, 2025
72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joao-r-reis joao-r-reis joao-r-reis 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.

2 participants

@tengu-alt @joao-r-reis