gateway e2e: eg install and uninstall test

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.

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

Fixes #3323

Jun 02 '24 10:06 ShyunnY

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 67.44%. Comparing base (a2e9bb5) to head (aefac57). Report is 1109 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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

:snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Jun 02 '24 11:06 codecov[bot]

/retest

Jun 02 '24 11:06 ShyunnY

/retest

Jun 05 '24 06:06 ShyunnY

/retest

Jun 20 '24 02:06 ShyunnY

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

Jun 25 '24 17:06 guydc

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

sounds good!

Jun 26 '24 01:06 ShyunnY

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

Jun 26 '24 11:06 guydc

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 :)

Jun 26 '24 14:06 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?

Jun 28 '24 16:06 ShyunnY

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

Jun 28 '24 21:06 guydc

/retest

Jul 01 '24 08:07 ShyunnY

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

Jul 04 '24 10:07 ShyunnY

/retest

Jul 04 '24 10:07 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.

Jul 05 '24 13:07 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?

Jul 05 '24 13:07 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?

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.

Jul 05 '24 17:07 guydc

@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.

Jul 06 '24 14:07 ShyunnY

/retest

Jul 07 '24 15:07 ShyunnY

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

Jul 14 '24 13:07 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.

Jul 21 '24 15:07 ShyunnY

/retest

Jul 22 '24 01:07 ShyunnY

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!

Aug 21 '24 04:08 github-actions[bot]

closing this PR since its become inactive, feel free to reopen if you're still working on it

May 23 '25 02:05 arkodg