test: support backward compatiblity checks in e2e testing #1656

bermuell · 2024-02-22T13:53:02Z

Description

Closes: #1559

Add backward compatibility tests to e2e testing

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Included the correct type prefix in the PR title
Targeted the correct branch (see PR Targeting)
Provided a link to the relevant issue or specification
Reviewed "Files changed" and left comments if necessary
Confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

Confirmed the correct type prefix in the PR title
Confirmed all author checklist items have been addressed
Confirmed that this PR does not change production code

fix gosec + refactor

…iblity-tests

…config

tests/e2e/main.go

tests/e2e/test_driver.go

p-offtermatt

Thanks for the work! Mostly looks good, left a couple of comments. I think in particular we need a bit more documentation on this.

I think a lot of this means extra work when we do new releases, and this will for sure be an extra maintenance burden - I'd suggest thinking of ways we can make this e.g. fail visibly when we make a new release and something hasn't been updated properly on the compatibility tests, e.g. somewhere the new release is not set as the latest version

Makefile

.github/workflows/nightly-e2e.yml

Dockerfile.combined

tests/e2e/actions.go

tests/e2e/config.go

p-offtermatt · 2024-02-27T09:27:50Z

tests/e2e/config.go

+		fmt.Println("Using old validator configs: ", providerVersion)
+
+		validatorCfg = map[ValidatorID]ValidatorConfig{
+			ValidatorID("alice"): {


I wonder if there is some common kernel that we should try to pull out here to reduce duplication, but maybe it's not possible or worth it - just from looking at a diffchecker, a few things seem to be common among versions, but maybe not enough to justify effort to unify it

tests/e2e/test_runner.go

p-offtermatt

LGTM, thanks for addressing the comments.

I think this would benefit from having someone who is more involved in the release process take a look. I am slightly concerned this introduces a lot of changes when we release a new version, but I don't really have the experience to say.
@MSalopek @mpoke I think one of you should probably have a look

mpoke · 2024-02-28T14:41:37Z

I am slightly concerned this introduces a lot of changes when we release a new version, but I don't really have the experience to say.

@p-offtermatt This PR doesn't modify any production code, thus it doesn't affect the release process.

MSalopek · 2024-02-29T09:13:28Z

LGTM, thanks for addressing the comments.

I think this would benefit from having someone who is more involved in the release process take a look. I am slightly concerned this introduces a lot of changes when we release a new version, but I don't really have the experience to say. @MSalopek @mpoke I think one of you should probably have a look

It does not affect releasing. There could be further changes for later versions and we may need to revisit the compatility matrix (some versions will not be complatible because of test initialization, differenc CLI comands used etc.)

p-offtermatt · 2024-02-29T09:19:55Z

LGTM, thanks for addressing the comments.
I think this would benefit from having someone who is more involved in the release process take a look. I am slightly concerned this introduces a lot of changes when we release a new version, but I don't really have the experience to say. @MSalopek @mpoke I think one of you should probably have a look

It does not affect releasing. There could be further changes for later versions and we may need to revisit the compatility matrix (some versions will not be complatible because of test initialization, differenc CLI comands used etc.)

Yeah, the points you made are what I was wondering about, not the release process itself, probably didn't express that clearly.

@sainoe

* Infra for compatibility tests * Dockerfile fix for hermes * cleanup fix gosec + refactor * adapt actions to target * Cleanup docker files * log cleanup * addressed comments * fix image cleanup * addressed PR comments * some more gosec fixes * ignore errors when trying to remove non-existing containers * updated e2e README * addressed remaining linter warning + caching & worktree improvement * docker image: use cometmock from provider image * fix testscript path * Addressed comments from @sainoe * Added compatibility tests * Added support using images from GH registry. Added version dependent config * Added test report + fixes * nit * addressed gosec issues + nit * try Makefile fix * Fix makefile * Addressed comments * added comment for transform parameter identification

bermuell added 21 commits January 10, 2024 11:57

Infra for compatibility tests

caf2214

Dockerfile fix for hermes

5ad3b22

cleanup

7b9bbb8

fix gosec + refactor

adapt actions to target

15e007e

Cleanup docker files

cb8b83f

log cleanup

9724657

addressed comments

b758c2e

fix image cleanup

a6e5820

addressed PR comments

45eabc5

some more gosec fixes

7db57bd

Merge remote-tracking branch 'origin/main' into bernd/1133-e2e-compat…

6c3e214

…iblity-tests

ignore errors when trying to remove non-existing containers

8d46f31

updated e2e README

a4065df

addressed remaining linter warning + caching & worktree improvement

42b00f4

docker image: use cometmock from provider image

786b1ad

fix testscript path

7f0105d

Addressed comments from @sainoe

f6055c6

Merge remote-tracking branch 'origin/main' into bernd/1133-e2e-compat…

9a07246

…iblity-tests

Merge remote-tracking branch 'origin/main' into bernd/1133-e2e-compat…

550e82a

…iblity-tests

Added compatibility tests

2ef6613

Added support using images from GH registry. Added version dependent …

d21ca86

…config

github-actions bot added C:Testing Assigned automatically by the PR labeler C:Build Assigned automatically by the PR labeler labels Feb 22, 2024

bermuell requested a review from p-offtermatt February 22, 2024 13:54

bermuell self-assigned this Feb 22, 2024

github-actions bot added the C:CI Assigned automatically by the PR labeler label Feb 26, 2024

bermuell force-pushed the bernd/1559-compatibility-tests branch 3 times, most recently from 854f3a4 to f2ba52b Compare February 26, 2024 16:46

github-advanced-security bot found potential problems Feb 26, 2024

View reviewed changes

tests/e2e/main.go Dismissed Show dismissed Hide dismissed

tests/e2e/test_driver.go Dismissed Show dismissed Hide dismissed

bermuell force-pushed the bernd/1559-compatibility-tests branch from f2ba52b to d0d88d1 Compare February 26, 2024 16:59

bermuell added 2 commits February 26, 2024 19:15

Merge remote-tracking branch 'origin/main' into 1559-compatibility-tests

38532f9

nit

7b419f4

bermuell force-pushed the bernd/1559-compatibility-tests branch from 7f73e92 to bebe958 Compare February 27, 2024 07:29

addressed gosec issues + nit

835c3cd

bermuell force-pushed the bernd/1559-compatibility-tests branch from bebe958 to 835c3cd Compare February 27, 2024 07:43

bermuell marked this pull request as ready for review February 27, 2024 07:44

bermuell requested a review from a team as a code owner February 27, 2024 07:44

bermuell requested a review from MSalopek February 27, 2024 07:44

try Makefile fix

f15cde8

p-offtermatt reviewed Feb 27, 2024

View reviewed changes

bermuell force-pushed the bernd/1559-compatibility-tests branch 5 times, most recently from 894e591 to f15cde8 Compare February 27, 2024 13:26

bermuell added 2 commits February 27, 2024 14:50

Fix makefile

946e0bc

Addressed comments

0760c47

bermuell force-pushed the bernd/1559-compatibility-tests branch from c882f59 to 0760c47 Compare February 28, 2024 12:45

p-offtermatt approved these changes Feb 28, 2024

View reviewed changes

added comment for transform parameter identification

44192d6

MSalopek approved these changes Feb 29, 2024

View reviewed changes

bermuell added this pull request to the merge queue Feb 29, 2024

Merged via the queue into main with commit 6ca97e8 Feb 29, 2024
18 checks passed

bermuell deleted the bernd/1559-compatibility-tests branch February 29, 2024 09:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: support backward compatiblity checks in e2e testing #1656

test: support backward compatiblity checks in e2e testing #1656

bermuell commented Feb 22, 2024 •

edited

Loading

p-offtermatt left a comment •

edited

Loading

p-offtermatt Feb 27, 2024

p-offtermatt left a comment

mpoke commented Feb 28, 2024

MSalopek commented Feb 29, 2024

p-offtermatt commented Feb 29, 2024

test: support backward compatiblity checks in e2e testing #1656

test: support backward compatiblity checks in e2e testing #1656

Conversation

bermuell commented Feb 22, 2024 • edited Loading

Description

Author Checklist

Reviewers Checklist

p-offtermatt left a comment • edited Loading

Choose a reason for hiding this comment

p-offtermatt Feb 27, 2024

Choose a reason for hiding this comment

p-offtermatt left a comment

Choose a reason for hiding this comment

mpoke commented Feb 28, 2024

MSalopek commented Feb 29, 2024

p-offtermatt commented Feb 29, 2024

bermuell commented Feb 22, 2024 •

edited

Loading

p-offtermatt left a comment •

edited

Loading