Skip to content

feat: remove gocheck from test suites#1883

Merged
kian99 merged 17 commits intocanonical:v3from
kian99:remove-all-go-check
Feb 25, 2026
Merged

feat: remove gocheck from test suites#1883
kian99 merged 17 commits intocanonical:v3from
kian99:remove-all-go-check

Conversation

@kian99
Copy link
Contributor

@kian99 kian99 commented Feb 24, 2026 •
edited by atlassian bot
Loading

Description

This PR does a few things, particularly it:

  1. Removes the JujuConnSuite, this was a simple ~50 line removal since nothing was using it.
  2. Refactors the test suites away from gocheck to use quicktest instead.
  3. Refactors the test suites away to move away from the suite model in line with the above move away from gocheck.

You will notice any test that relied on gocheck and our integration suites has had very similar changes.

  • All checkers like gc.Equal are now qt.Equal.
  • Structs like type fooSuite struct have been removed if unneeded or adapted into a small SetupFooTests() function that is called at the start of each test that needs the custom setup.
  • Refactored tests now call c := qt.New(t) and s := jimmtest.SetupJimmEnv(c) or some variant at the start of each test to setup the integration test environment.

There are still some trace uses of gocheck around the codebase used in unit tests that I will get rid of in a follow-up.

Fixes JUJU-9240

Engineering checklist

  • Documentation updated
  • Covered by unit tests
  • Covered by integration tests

@kian99 kian99 requested a review from a team as a code owner February 24, 2026 07:15
@kian99 kian99 changed the title feat: refactor test suites feat: remove gocheck from test suites Feb 24, 2026
@kian99 kian99 force-pushed the remove-all-go-check branch 2 times, most recently from 8bbad39 to afed1b4 Compare February 24, 2026 11:46
@kian99 kian99 force-pushed the remove-all-go-check branch from ec26c87 to d2b0e6d Compare February 24, 2026 12:51
alesstimec
alesstimec approved these changes Feb 24, 2026
View reviewed changes
SimoneDutto
SimoneDutto reviewed Feb 24, 2026
View reviewed changes
Copy link
Contributor

@SimoneDutto SimoneDutto left a comment

Choose a reason for hiding this comment

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

i think we can row get rid of juju-db from the sys-deps in the makefile right?

Are we are missing the cleanup for each test? Or i've just missed it?

@kian99
Copy link
Contributor Author

kian99 commented Feb 25, 2026

@SimoneDutto The change to use --replay fixed that debug-log test. Only 1 more test is failing now, TestOffer, where it's hitting a timeout after 5 minutes trying to cleanup the model. Not sure why though.

@SimoneDutto
Copy link
Contributor

SimoneDutto commented Feb 25, 2026

@kian99 one question:
I see every test starts with jimmtest.SetupWebsocketEnv(c) but i don't see the corresponding cleanup. I believe that each test creates a model, and cloud credentials and other stuff on the backing controller we never clean up?

@kian99
Copy link
Contributor Author

kian99 commented Feb 25, 2026

@kian99 one question: I see every test starts with jimmtest.SetupWebsocketEnv(c) but i don't see the corresponding cleanup. I believe that each test creates a model, and cloud credentials and other stuff on the backing controller we never clean up?

When we create a model in the suite, we call c.Cleanup(<cleanup-call>) so the cleanup gets registered for the end of the test. Essentially you write the cleanup as you write the setup, with c.Cleanup() acting like a defer statement.

@SimoneDutto
Copy link
Contributor

SimoneDutto commented Feb 25, 2026

@kian99 one question: I see every test starts with jimmtest.SetupWebsocketEnv(c) but i don't see the corresponding cleanup. I believe that each test creates a model, and cloud credentials and other stuff on the backing controller we never clean up?

When we create a model in the suite, we call c.Cleanup(<cleanup-call>) so the cleanup gets registered for the end of the test. Essentially you write the cleanup as you write the setup, with c.Cleanup() acting like a defer statement.

aaaa, thanks!

SimoneDutto
SimoneDutto approved these changes Feb 25, 2026
View reviewed changes
Copy link
Contributor

@SimoneDutto SimoneDutto left a comment

Choose a reason for hiding this comment

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

thanks for the effort!

@kian99 kian99 merged commit 651a5b5 into canonical:v3 Feb 25, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alesstimec alesstimec alesstimec approved these changes

@SimoneDutto SimoneDutto SimoneDutto 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

@kian99 @SimoneDutto @alesstimec