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

chore: e2e improve docker cache #16

Merged
merged 38 commits into from
Aug 9, 2024
Merged

Conversation

RafilxTenfen
Copy link
Contributor

@RafilxTenfen RafilxTenfen commented Aug 8, 2024

  • Add multiple CI jobs to run e2e
  • e2e-docker-build-* builds docker and saves it as an artifact
  • e2e-run-* runs each e2e test individually
  • remove the need of sudo on running e2e tests due to removing error check in ClearResources 🥲
    it errors out due to unlinkat /tmp/bbn-e2e-testnet-2820217771/bbn-test-a/bbn-test-a-node-babylon-default-a-2/ibc_08-wasm/state/wasm: permission denied
  • removing the need of sudo to run e2e speeds up the run because it uses the go cache created from job build

TLDR; It is not a pretty solution, but reduces the CI running time of e2e

Accepting suggestions/inputs to make it better

image

✔️ CI time reduced to 8min

@RafilxTenfen RafilxTenfen self-assigned this Aug 8, 2024
…io/babylon into rafilx/e2e-improve-docker-cache
@RafilxTenfen RafilxTenfen marked this pull request as ready for review August 8, 2024 18:14
@RafilxTenfen RafilxTenfen changed the base branch from rafilx/e2e-improve to dev August 8, 2024 18:15
Copy link
Collaborator

@liam-icheng-lai liam-icheng-lai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

Copy link
Collaborator

@KonradStaniec KonradStaniec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question, otherwise LGTM! great work!

contrib/images/babylond/Dockerfile Outdated Show resolved Hide resolved
@RafilxTenfen RafilxTenfen merged commit 468bbae into dev Aug 9, 2024
17 checks passed
RafilxTenfen added a commit that referenced this pull request Aug 9, 2024
Add upgrade to insert BTC headers

Should be merged after #16
@filippos47 filippos47 deleted the rafilx/e2e-improve-docker-cache branch August 12, 2024 08:27
vitsalis pushed a commit that referenced this pull request Aug 14, 2024
- Add multiple CI jobs to run e2e
- `e2e-docker-build-*` builds docker and saves it as an artifact
- `e2e-run-*` runs each e2e test individually
- remove the need of sudo on running e2e tests due to removing error
check in ClearResources 🥲
it errors out due to `unlinkat
/tmp/bbn-e2e-testnet-2820217771/bbn-test-a/bbn-test-a-node-babylon-default-a-2/ibc_08-wasm/state/wasm:
permission denied`
- removing the need of sudo to run e2e speeds up the run because it uses
the go cache created from job `build`

TLDR; It is not a pretty solution, but reduces the CI running time of
e2e
> Accepting suggestions/inputs to make it better 


![image](https://github.com/user-attachments/assets/acfb51e9-d09e-4e60-b384-34fc11c0aa41)



:heavy_check_mark: CI time reduced to 8min
vitsalis pushed a commit that referenced this pull request Aug 14, 2024
Add upgrade to insert BTC headers

Should be merged after #16
@@ -40,12 +40,11 @@ func (cb *CurrentBranchConfigurer) ConfigureChains() error {

func (cb *CurrentBranchConfigurer) ConfigureChain(chainConfig *chain.Config) error {
cb.t.Logf("starting e2e infrastructure from current branch for chain-id: %s", chainConfig.Id)
tmpDir, err := os.MkdirTemp("", "bbn-e2e-testnet-")
tmpDir, err := os.MkdirTemp("", "bbn-e2e-testnet-*")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this? Just curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// If pattern includes a "*", the random string replaces the last "*" instead.
func MkdirTemp(dir, pattern string) (string, error) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

},
withUpgrade(baseSetup), // base set up with upgrade
containerManager,
upgradePath,
0,
), nil
}

func identifierName(t *testing.T) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? Just curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you would run two or more tests at the same time they might match the name of the container and mess up everything 🤯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants