e2e: eg install and uninstall test

[ShyunnY]

What type of PR is this?
e2e: eg install and uninstall test

What this PR does / why we need it:

Which issue(s) this PR fixes:
This PR adds e2e tests for the parts of Helm.PackageTool used in egctl.

1. I set both install/uninstall to the same ConformanceTest. This avoids duplicating the install/uninstall process for separate tests
2. Minor refactoring of the upgrade part, which I think belongs to the package manager feature.

Fixes #3323

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.44%. Comparing base (a2e9bb5) to head (aefac57).
Report is 91 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3515      +/-   ##
==========================================
+ Coverage   67.36%   67.44%   +0.08%     
==========================================
  Files         182      182              
  Lines       22433    22433              
==========================================
+ Hits        15111    15130      +19     
+ Misses       6230     6216      -14     
+ Partials     1092     1087       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ShyunnY]

/retest

[ShyunnY]

/retest

[ShyunnY]

/retest

[guydc]

maybe use the term lifecycle instead of package/upgrade? It's slightly broader.

[ShyunnY]

maybe use the term lifecycle instead of package/upgrade? It's slightly broader.

sounds good!

[guydc]

Hi @ShyunnY ! should we also update the existing "upgrade" test to use the new helm util?

[ShyunnY]

Hi @ShyunnY ! should we also update the existing "upgrade" test to use the new helm util?

@guydc
I'm not sure if what you mean by "update the existing "upgrade" test to use the new helm util" is: we should modify the logic in e2e/upgrade.go to use the tool in internal/utils/helm/package.go for the upgrade.
Hopefully my understanding is correct :)

[ShyunnY]

@guydc

I thought about it, let's solve the current PR first. In the future I will open an issue: about adding egctl upgrade feature, I will enhance helm util and let upgrade e2e use helm util.
What do you think?

[guydc]

yes, sure, let's get this in first.

[ShyunnY]

/retest

[ShyunnY]

hi, @arkodg
I'm sorry for the delay in finishing this PR, I've done!

[ShyunnY]

/retest

[guydc]

In addition, we specify the order of execution in the Makefile, will this still affect other test suites?

It will not impact other suites, but may impact other tests within this suite.

[ShyunnY]

In addition, we specify the order of execution in the Makefile, will this still affect other test suites?

It will not impact other suites, but may impact other tests within this suite.

can you give an example? Thanks!
Or can we specify the order in which the suites are executed?

[guydc]

In addition, we specify the order of execution in the Makefile, will this still affect other test suites?

It will not impact other suites, but may impact other tests within this suite.

can you give an example? Thanks! Or can we specify the order in which the suites are executed?

One example that comes to mind:

• Shutdown Test: if Uninstall/Install test executes before shutdown test, shutdown test will run on v0.0.0-latest instead of main. A regression in the shutdown manager component may go undiscovered in the PR, as main is not deployed during that test.

I think that we can just seperate this test into another suite and ensure that it runs after everything else in the run-e2e make target, or explicitly control the order of tests in the upgrade/lifecycle suite.

[ShyunnY]

@guydc

wow, I like this solution ^_^

IMO, we should try not to introduce unnecessary complexity in E2E testing, which may cause additional confusion for new users. I will take your solution and change it, thank you for your review.

[ShyunnY]

/retest

[ShyunnY]

It seems like we've been procrastinating for a long time, can we please get on with this work? :)

[shawnh2]

LGTM! Probably need to wait #3764 gets in first.

[ShyunnY]

I'm sorry for delaying the resolution of this conflict.

In the latest commit, I thought to simplify the lifecycle process as much as possible. So I executed it as a separate E2E Test and scheduled it to be executed in the last task. Avoid affecting the rest of the E2E.

[ShyunnY]

/retest

[github-actions]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions!