Skip to content
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

Refactor controller tests into separate suites #3304

Closed
danail-branekov opened this issue May 27, 2024 · 0 comments
Closed

Refactor controller tests into separate suites #3304

danail-branekov opened this issue May 27, 2024 · 0 comments
Assignees
@danail-branekov

Comments

@danail-branekov
Copy link
Member

danail-branekov commented May 27, 2024 •
edited
Loading

We have realised that having a single controller/webhook in a suite is quite helpful for testing:

  • There is no noise from neighbour controllers
  • Significantly simplifies the test as it does not need to always create valid dependend objects
  • Significantly simplifies the suite setup as the suite only installs a single controller
@danail-branekov danail-branekov self-assigned this May 27, 2024
danail-branekov added a commit that referenced this issue May 27, 2024
@danail-branekov
Split domain and route controllers into own packages
4a6861c 
issue: #3304
danail-branekov added a commit that referenced this issue May 27, 2024
@danail-branekov
Split domain and route controllers into own packages
0472b0b 
issue: #3304
danail-branekov added a commit that referenced this issue May 27, 2024
@danail-branekov
Replace CFRoute Valid/Invalid with the Ready condition
9d20235 
The `Ready` status condition is becoming the standard for Korifi
resources, therefore let's get rid of the `Valid` and `Invalid`
conditions in favour of the `Ready` one. Thus the CFRoute status
reconciliation code could be significantly simplified as we already have
the readyConditionBuilder utility in place.

Noone seems to care about `Valid`/`Invalid` status conditions, noone
cares about the `Ready` condition as well. However, having a standard
`Ready` condition is good (see #2098)

While being here, `CFRoute.Status.Description` and
`CFRoute.Status.CurrentStatus` are removed (as
again, noone cares) - hopefully this is not an incompatible change. If
anyone ever gets interested whether the route is OK, they should check
the `Ready` condition instead.

issue: #3304
@danail-branekov danail-branekov mentioned this issue May 27, 2024
Merged
tcdowney pushed a commit that referenced this issue May 29, 2024
@danail-branekov @tcdowney
Split domain and route controllers into own packages
802eadd 
issue: #3304
tcdowney pushed a commit that referenced this issue May 29, 2024
@danail-branekov @tcdowney
Replace CFRoute Valid/Invalid with the Ready condition
4ba96b3 
The `Ready` status condition is becoming the standard for Korifi
resources, therefore let's get rid of the `Valid` and `Invalid`
conditions in favour of the `Ready` one. Thus the CFRoute status
reconciliation code could be significantly simplified as we already have
the readyConditionBuilder utility in place.

Noone seems to care about `Valid`/`Invalid` status conditions, noone
cares about the `Ready` condition as well. However, having a standard
`Ready` condition is good (see #2098)

While being here, `CFRoute.Status.Description` and
`CFRoute.Status.CurrentStatus` are removed (as
again, noone cares) - hopefully this is not an incompatible change. If
anyone ever gets interested whether the route is OK, they should check
the `Ready` condition instead.

issue: #3304
danail-branekov added a commit that referenced this issue May 29, 2024
@danail-branekov
Split domain and route controllers into own packages
9373c92 
issue: #3304
danail-branekov added a commit that referenced this issue May 29, 2024
@danail-branekov
Replace CFRoute Valid/Invalid with the Ready condition
89504cc 
The `Ready` status condition is becoming the standard for Korifi
resources, therefore let's get rid of the `Valid` and `Invalid`
conditions in favour of the `Ready` one. Thus the CFRoute status
reconciliation code could be significantly simplified as we already have
the readyConditionBuilder utility in place.

Noone seems to care about `Valid`/`Invalid` status conditions, noone
cares about the `Ready` condition as well. However, having a standard
`Ready` condition is good (see #2098)

While being here, `CFRoute.Status.Description` and
`CFRoute.Status.CurrentStatus` are removed (as
again, noone cares) - hopefully this is not an incompatible change. If
anyone ever gets interested whether the route is OK, they should check
the `Ready` condition instead.

issue: #3304
@danail-branekov danail-branekov mentioned this issue May 30, 2024
Open
danail-branekov added a commit that referenced this issue May 30, 2024
@danail-branekov
Split controllers and webhooks into separate packages
3570f5b 
Having controllers in separate packages allows having a suite per a
controller. This has several benefits:
* The suite setup is significantly simpler - it does not have to wire
  all the controllers/webhooks. Instead, the setup wires only the
  controller/webhook that is being tested
* Not wiring all the controllers in the test env reduces the noise of
  unrelated controllers/webhooks. This allows tests to not care about
  creating "valid" dependent objects, they only create dependent
  objects with properties that are just relevant to the
  controller/webhook under test
* Tests are no longer required to take side effects from neighbour
  controllers/webhooks into consideration
* Instead of relying on a state change of a dependent object caused by a
  neightbour controller, the test could simply set the desired state of
  the dependent object. This makes the test simpler and easy to reason
  about

fixes #3304

wip

wip: remove GenerateGUID

wip

wip
danail-branekov added a commit that referenced this issue May 30, 2024
@danail-branekov
Split controllers and webhooks into separate packages
aa2d881 
Having controllers in separate packages allows having a suite per a
controller. This has several benefits:
* The suite setup is significantly simpler - it does not have to wire
  all the controllers/webhooks. Instead, the setup wires only the
  controller/webhook that is being tested
* Not wiring all the controllers in the test env reduces the noise of
  unrelated controllers/webhooks. This allows tests to not care about
  creating "valid" dependent objects, they only create dependent
  objects with properties that are just relevant to the
  controller/webhook under test
* Tests are no longer required to take side effects from neighbour
  controllers/webhooks into consideration
* Instead of relying on a state change of a dependent object caused by a
  neightbour controller, the test could simply set the desired state of
  the dependent object. This makes the test simpler and easy to reason
  about

fixes #3304
@danail-branekov danail-branekov mentioned this issue May 30, 2024
Merged
danail-branekov added a commit that referenced this issue May 31, 2024
@danail-branekov
Split controllers and webhooks into separate packages
0c1c260 
Having controllers in separate packages allows having a suite per a
controller. This has several benefits:
* The suite setup is significantly simpler - it does not have to wire
  all the controllers/webhooks. Instead, the setup wires only the
  controller/webhook that is being tested
* Not wiring all the controllers in the test env reduces the noise of
  unrelated controllers/webhooks. This allows tests to not care about
  creating "valid" dependent objects, they only create dependent
  objects with properties that are just relevant to the
  controller/webhook under test
* Tests are no longer required to take side effects from neighbour
  controllers/webhooks into consideration
* Instead of relying on a state change of a dependent object caused by a
  neightbour controller, the test could simply set the desired state of
  the dependent object. This makes the test simpler and easy to reason
  about

fixes #3304
danail-branekov added a commit that referenced this issue May 31, 2024
@danail-branekov
Split controllers and webhooks into separate packages
329ee72 
Having controllers in separate packages allows having a suite per a
controller. This has several benefits:
* The suite setup is significantly simpler - it does not have to wire
  all the controllers/webhooks. Instead, the setup wires only the
  controller/webhook that is being tested
* Not wiring all the controllers in the test env reduces the noise of
  unrelated controllers/webhooks. This allows tests to not care about
  creating "valid" dependent objects, they only create dependent
  objects with properties that are just relevant to the
  controller/webhook under test
* Tests are no longer required to take side effects from neighbour
  controllers/webhooks into consideration
* Instead of relying on a state change of a dependent object caused by a
  neightbour controller, the test could simply set the desired state of
  the dependent object. This makes the test simpler and easy to reason
  about

fixes #3304
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@danail-branekov danail-branekov

Labels
None yet
Projects
Korifi - Backlog
Archived in project
Milestone
No milestone
Development

No branches or pull requests
1 participant
@danail-branekov