integration-tests/smoke: add plugins variant to TestOCRv2Basic

[jmank88]

No description provided.

[github-actions]

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

[github-actions]

I see that you haven't updated any README files. Would it make sense to do so?

[ilija42]

Does this affect CI? Not completely sure how these smoke tests are ran, IIUC it uses this same smoke test for plugin and non plugin variant, but just builds it differently?

[jmank88]

Does this affect CI? Not completely sure how these smoke tests are ran, IIUC it uses this same smoke test for plugin and non plugin variant, but just builds it differently?

Yes, you can see the subtest in the logs for the ocr2-* jobs. There is a -plugins variant already, but that is running a different dockerfile, which has all plugins, and enables them by default. This test will execute on the regular docker file too, which has limited plugins, and disables them by default. This way we try the feature flag both ways on both images.

It does run more than necessary though, since there are so many ocr2-* variants for eth clients, so maybe there is a better way to organize this. 🤔
cc @kalverra

[kalverra]

It does run more than necessary though, since there are so many ocr2-* variants for eth clients, so maybe there is a better way to organize this. 🤔

These are new additions that I'm currently working on moving to a nightly job instead of every PR. If you feel the plugins tests need a similar treatment we can move them to that style as well.

I would prefer to make it a little more explicit what this is meant to do regarding running on a regular image with feature flags shut off. Maybe make a comment along with renaming the sub test to with-disabled-plugins or something?

[jmank88]

These are new additions that I'm currently working on moving to a nightly job instead of every PR. If you feel the plugins tests need a similar treatment we can move them to that style as well.

It is important to run on every PR, but only once. So maybe this is fine as-is, but could wait for that PR first?

I would prefer to make it a little more explicit what this is meant to do regarding running on a regular image with feature flags shut off. Maybe make a comment along with renaming the sub test to with-disabled-plugins or something?

Can you elaborate?
The sub-tests are named legacy and plugins which is a pattern we've established in a few places. The regular image has the feature disabled already, so legacy env vars are a no-op, but plugins validates that we can enable the feature. The -plugins image has it enabled by default, so legacy disables and plugins is a no-op.

(we will be able to drop the -plugins image eventually - hopefully in Q1)

[kalverra]

It is important to run on every PR, but only once. So maybe this is fine as-is, but could wait for that PR first?

By "it" here, do you mean the plugins tests? Or the client tests?

(we will be able to drop the -plugins image eventually - hopefully in Q1)

Ah, okay, never mind then.

[jmank88]

It is important to run on every PR, but only once. So maybe this is fine as-is, but could wait for that PR first?

By "it" here, do you mean the plugins tests? Or the client tests?

I mean plugins.

Clients are less significant, although we could probably trigger it for certain chains/evm/clients paths or whatever is related 🤷

[kalverra]

Clients are less significant, although we could probably trigger it for certain chains/evm/clients paths or whatever is related

We're on the same page then. I'll keep it to just nightly for now, unless you have some specific paths in mind at the moment.

[jmank88]

Rebased since merge conflicts were a mess.

[cl-sonarqube-production]

SonarQube Quality Gate

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication