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.

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

Restore support for running custom events #177

Closed
tonyandrewmeyer opened this issue Aug 26, 2024 · 3 comments
Closed

Restore support for running custom events #177

tonyandrewmeyer opened this issue Aug 26, 2024 · 3 comments
Labels
25.04 enhancement New feature or request

Comments

@tonyandrewmeyer
Copy link
Collaborator

tonyandrewmeyer commented Aug 26, 2024

In Scenario 6, it's possible to run custom events:

ctx = Context(...)
state_in = scenario.State()
state_out = ctx.run("my_charm_lib.on.foo", state_in)
assert state_out...

In Scenario 7.0, this has been removed: firstly because we got rid of the 'stringly-typed' events, and, more significantly, because we felt that the level of abstraction for Scenario is at the "Juju event". The intention was that people would run the Juju event that would cause the custom event to be emitted, e.g. ctx.on.relation_changed() might end up causing a DatabaseCreated event to be emitted.

@gruyaume pushed back on this:

our charm unit tests shouldn't know about library specificities. Our charm does not look into the relation databags, it only interfaces with the library. Taking the approach you are proposing entails our charm unit test to replicate the behavior of libraries that we don't have ownership over.

I feel like the charm author probably does need to know which Juju events will cause the custom events to be able to manually use the charm and to be able to write integration tests, and that you shouldn't need to manage the relation databags.

However, there are potentially many events and you have to know which ones might be triggered for what's happening. For example, with the data_platform DatabaseRequires class from data-platform-libs, it probably needs something like:

def test_credentials():
    ctx = Context(DemoAPICharm)
    db_rel = Relation("database")
    container = Container("webapp", can_connect=True)
    state = State(relations={db_rel})
    state = ctx.run(ctx.on.relation_created(), state)
    state = ctx.run(ctx.on.relation_joined(), state)
    state = ctx.run(ctx.on.relation_changed(), state)
    state = ctx.run(ctx.on.secret_changed(), state)

    assert state.get_container("webapp")... # check that the config file is there or whatever

Maybe tests that rely on custom events should be Catan tests? I think what I want to be testing is that when I integrate the two charms the state of the charm I'm writing ends up the way I expect.

However, maybe we should add back some way to run custom events explicitly. @gruyaume also needed the ability to provide custom arguments for creating the event, which Scenario 6 can't do.

Maybe this looks like this?

ctx = Context(...)
state_in = State(...)
state_out = ctx.run(ctx.on.custom_event('database.on.database_ready', x, y=42), state_in)
assert state_out...

custom_event would be a static method on CharmEvents with a signature like (attribute: str, *args, **kwargs) and it would pass *args and **kwargs through to the custom event class.

@tonyandrewmeyer tonyandrewmeyer added the enhancement New feature or request label Aug 26, 2024
@dimaqq
Copy link
Collaborator

dimaqq commented Aug 29, 2024

my 2c / would it be amazing if...

Scenario 7 provided a way for charm libs to provide Scenario extensions, or maybe pytest fixtures that encapsulate the library API that's exposed to the charm?

I feel that test doubles for custom events fit squarely there.

P.S. perhaps two things/approaches could be offered:

  1. a lib mock that charm under test interacts with under Scenario
  2. a lib double that creates batches of events that charm+lib under test process under Catan

@tonyandrewmeyer
Copy link
Collaborator Author

Scenario 7 provided a way for charm libs to provide Scenario extensions, or maybe pytest fixtures that encapsulate the library API that's exposed to the charm?

Yeah, doing something like this had crossed my mind too, although there's a lot more to figure out, and then a bunch of buy-in needed.

@tonyandrewmeyer
Copy link
Collaborator Author

Moved to canonical/operator#1421

@tonyandrewmeyer tonyandrewmeyer closed this as not planned Won't fix, can't repro, duplicate, stale Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
25.04 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants