local-env-governance-utxo-with-plutus-script

[rsporny]

added:

• governance authority in local env has plutus script attached to mimic real life scenarios

Refs: ETCM-9630

[LGLO]

I think it should be a test case in e2e-tests:

• given an utxo with script attached
• when governance init is run for such case
• then init completes successfully

How many test scenarios are we going to squeeze into local-env setup?

EDIT: if I revert my fix, I should have one test case failed. But if this is added to local-env setup, then almost all tests will be failed.

[rsporny]

@LGLO you've already covered it in unit tests, so it won't even get pass through cargo check. What you're proposing is not e2e test, because it would only test command's response and/or main chain state. There is no e2e test for governance init because chain builder is interested in starting a chain, governance init is only one of the needed steps to achieve this. Current e2e test coverage is implicitly testing it in local-env entrypoint.

What you're asking for is an integration level coverage which we can address separately. For now, it's not a bad idea to mimic real-life scenarios while we set up local-env. In case of failure the reason may not be clear, but it's a clear win-win solution, because we'll catch this bug (and probably others too) in CI.

[LGLO]

Current e2e test coverage is implicitly testing it in local-env entrypoint.
Yes, and we have obviously 1 entry point that calls governance init, upsert-d-parameter, upsert-permissioned-candidates all of them with single key governance.

governance init is "implicitly tested" with a UTXO that has no script attached.
This pull request changes this to implicitly testing governance init with UTXO that has Plutus Script attached.

The bad news is that this won't cover scenario where UTXO has Native Script attached. I can assure you that we have a bug with governance init using genesis utxo with a native script attached. So, how are you going to test both case with this approach?

If e2e-tests are going to cover multiple scenarios for governance init: no script, PlutusScript, NativeScript, multi-assets present or not, then governance init should be called from e2e-tests.
If e2e-tests are only interested in one of the choices, let it be the simplest choice and then we can either settle on adding unit tests (asserting of lack of failures, or having fee over some value) or we can add Rust integration tests that assert us that governance init transaction is successful.

[rsporny]

@LGLO The approach to test such cases is to use integration tests. e2e-tests are not going to cover all of the above, but as I said, there is nothing wrong with enhancing initial setup with additional data, because the goal is to catch bugs. It serves now as a second safety net. Are you suggesting that we should not increase our coverage now (therefore increase the risk of unnoticed bugs) and wait until we have integration tests, because you don't like the fact that we're testing governance init indirectly?

[LGLO]

I'm saying local-env is not a place to make changes when we need another test scenario. We can merge this change now. The next day we'll find out that it does not cover ETCM-9632 and we do another update? And as I said before, this way we can only test one case. I do not block this PR and I don't want to be complicit in going in wrong direction.

[LGLO]

This UTOX does not have script attached.