Add complete test cases for SparkApplication #7087

Abhay349 · 2026-01-05T14:39:58Z

What does this PR do?

This PR adds test cases for
SparkApplication (sparkoperator.k8s.io/v1beta2).

gemini-code-assist · 2026-01-05T14:40:21Z

Summary of Changes

Hello @Abhay349, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the test coverage for SparkApplication (sparkoperator.k8s.io/v1beta2) resource interpretation. It introduces new comprehensive test suites for InterpretComponent and InterpretHealth operations, ensuring robust validation of how Spark applications are represented and their health is determined. Additionally, existing test cases for AggregateStatus and InterpretStatus have been expanded to cover more edge cases and ensure correct behavior under various conditions, ultimately improving the reliability of Spark application management.

Highlights

New InterpretComponent Test Cases: Introduced a new test file for InterpretComponent of SparkApplication, covering scenarios with and without dynamic allocation, as well as non-JVM jobs to ensure correct component interpretation.
New InterpretHealth Test Cases: Added a new test file for InterpretHealth of SparkApplication, validating health status for completed, failed, and missing status scenarios to accurately reflect application health.
Expanded AggregateStatus Test Cases: Enhanced the AggregateStatus test suite to include cases for SparkApplications with a single status item and those with no status items, improving robustness for status aggregation.
Expanded InterpretStatus Test Cases: Updated the InterpretStatus test cases to cover situations where the observed status of a SparkApplication is empty or missing, ensuring proper handling of incomplete status data.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request significantly improves the test coverage for the SparkApplication resource interpreter by adding several new test cases for status aggregation, component interpretation, health checks, and status reflection. The new tests are well-structured and cover important edge cases. I've identified an issue with an incorrect expected memory value in one of the new test files and a minor formatting issue. Overall, this is a valuable contribution.

...izations/sparkoperator.k8s.io/v1beta2/SparkApplication/testdata/interpretcomponent-test.yaml

codecov-commenter · 2026-01-05T14:56:53Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.56%. Comparing base (2d6e8f8) to head (4a02960).
⚠️ Report is 9 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7087      +/-   ##
==========================================
+ Coverage   46.55%   46.56%   +0.01%     
==========================================
  Files         700      700              
  Lines       48091    48091              
==========================================
+ Hits        22387    22392       +5     
+ Misses      24021    24017       -4     
+ Partials     1683     1682       -1

Flag	Coverage Δ
unittests	`46.56% <ø> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

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

XiShanYongYe-Chang · 2026-01-06T12:35:05Z

Thanks
/assign

XiShanYongYe-Chang · 2026-01-06T12:43:45Z

Hi @FAUST-BENCHOU Would you like to review this PR?

FAUST-BENCHOU · 2026-01-06T12:50:00Z

Hi @FAUST-BENCHOU Would you like to review this PR?

Sure!let me take a look at relative lua script first.

FAUST-BENCHOU · 2026-01-06T13:40:24Z

The core tests are covered, but the boundary case tests are relatively weak. Below are some of my suggestions for improvement (of course, you can extend these suggestions as well).

AggregateStatus

1.Several status items have missing fields: Some untested items may lack fields such as executionAttempts and submissionAttempts.
2.The comparison and other logic for lastSubmissionAttemptTime and terminationTime.

karmada/pkg/resourceinterpreter/default/thirdparty/resourcecustomizations/sparkoperator.k8s.io/v1beta2/SparkApplication/customizations.yaml

Lines 302 to 305 in 04aed89

    
                         if currentStatus.lastSubmissionAttemptTime and (lastSubmissionAttemptTime == "" or currentStatus.lastSubmissionAttemptTime > lastSubmissionAttemptTime) then 
        
                           lastSubmissionAttemptTime = currentStatus.lastSubmissionAttemptTime 
        
                         end 
        
                         if currentStatus.terminationTime and (terminationTime == "" or currentStatus.terminationTime > terminationTime) then

InterpretComponent

1.GPU resources related

karmada/pkg/resourceinterpreter/default/thirdparty/resourcecustomizations/sparkoperator.k8s.io/v1beta2/SparkApplication/customizations.yaml

Lines 187 to 194 in 04aed89

    
                     local drv_gpu = get(observedObj, {"spec","driver","gpu"}) 
        
                     if drv_gpu ~= nil then 
        
                       local gpu_name = drv_gpu.name 
        
                       local gpu_qty = to_num(drv_gpu.quantity, 0) 
        
                       if not isempty(gpu_name) and gpu_qty > 0 then 
        
                         drv_requires.resourceRequest[gpu_name] = gpu_qty 
        
                       end 
        
                     end

2.some default value like executor.instances driver.memory executor.memory......
3.Pod template related

karmada/pkg/resourceinterpreter/default/thirdparty/resourcecustomizations/sparkoperator.k8s.io/v1beta2/SparkApplication/customizations.yaml

Lines 84 to 108 in 04aed89

    
                   local function apply_pod_template(pt_spec, requires) 
        
                     if pt_spec == nil then 
        
                       return 
        
                     end 
        
                     local nodeSelector = clone_plain(pt_spec.nodeSelector) 
        
                     local tolerations  = clone_plain(pt_spec.tolerations) 
        
                     local priority     = pt_spec.priorityClassName 
        
                     local hardNodeAffinity = nil 
        
                     if pt_spec.affinity and pt_spec.affinity.nodeAffinity then 
        
                       hardNodeAffinity = clone_plain(pt_spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution) 
        
                     end 
        
                     -- Only create nodeClaim if there is content 
        
                     if nodeSelector ~= nil or tolerations ~= nil or hardNodeAffinity ~= nil then 
        
                       requires.nodeClaim = requires.nodeClaim or {} 
        
                       requires.nodeClaim.nodeSelector = nodeSelector 
        
                       requires.nodeClaim.tolerations  = tolerations 
        
                       requires.nodeClaim.hardNodeAffinity = hardNodeAffinity 
        
                     end 
        
                     if not isempty(priority) then 
        
                       requires.priorityClassName = priority 
        
                     end 
        
                   end

InterpretHealth

all in all good.maybe you can also consider scenarios where state is an empty string or applicationState exists but the state field is missing.

InterpretStatus (ReflectStatus)

good but maybe u can consider that when applicationState, driverInfo, executorState, etc. are nil, a copy will be made.

@Abhay349 Thanks for you awesome job!

FAUST-BENCHOU · 2026-01-06T13:41:20Z

@XiShanYongYe-Chang This is just my opinion; any feedback is welcome.🙃

Abhay349 · 2026-01-06T14:09:23Z

@FAUST-BENCHOU Thanks for review..
let me look on those issues.

Abhay349 · 2026-01-06T14:30:27Z

Thanks a lot for the review and suggestions
I’m a beginner with these interpreter tests, so this feedback really helps.
I just wanted to confirm, for cases where some status fields are missing, should we add explicit tests for those scenarios or treat them as default values?

I’ll update the tests accordingly. Thanks again!

FAUST-BENCHOU · 2026-01-06T15:05:16Z

Thanks a lot for the review and suggestions I’m a beginner with these interpreter tests, so this feedback really helps. I just wanted to confirm, for cases where some status fields are missing, should we add explicit tests for those scenarios or treat them as default values?

I’ll update the tests accordingly. Thanks again!

I think we should explicitly test the boundary cases of missing fields, rather than relying solely on default values, to ensure the logic is correct when fields are missing.

Abhay349 · 2026-01-06T15:07:41Z

Sure, I will fix it shortly..
Thanks :)

Signed-off-by: Abhay349 <[email protected]>

Abhay349 · 2026-01-06T20:00:47Z

hi @FAUST-BENCHOU @XiShanYongYe-Chang, I have added required changes,
happy to iterate on further feedback..

Thanks

XiShanYongYe-Chang · 2026-01-07T01:54:47Z

Let's wait for @FAUST-BENCHOU to confirm it again.

FAUST-BENCHOU · 2026-01-07T02:24:47Z

@XiShanYongYe-Chang overall great!

XiShanYongYe-Chang

Thanks @Abhay349 @FAUST-BENCHOU
/lgtm
/approve

karmada-bot · 2026-01-07T02:47:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: XiShanYongYe-Chang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/resourceinterpreter/default/thirdparty/OWNERS~~ [XiShanYongYe-Chang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

FAUST-BENCHOU · 2026-01-07T02:03:58Z

...tomizations/sparkoperator.k8s.io/v1beta2/SparkApplication/testdata/interprethealth-test.yaml

+      state: ""
+operation: InterpretHealth
+output:
+  healthy: true


SparkApplication unhealthy when applicationState.state is empty
why is the output healthy?

According to logic,
empty string "" !="FAILED",so state == "" is treated as healthy

got that,maybe you can change comment to like SparkApplication healthy when applicationState.state is empty

FAUST-BENCHOU · 2026-01-07T02:05:38Z

...tomizations/sparkoperator.k8s.io/v1beta2/SparkApplication/testdata/interpretstatus-test.yaml

+    applicationState: {}
+operation: InterpretStatus
+output:
+  status: {}


The message is "InterpretStatus should copy applicationState even if the state field is missing".but the expected output is: status: {}. This is inconsistent with the description of "copy applicationState" (if it is to be copied, the applicationState field should be included, even if the map is empty).

Apologies for description issue,
"InterpretStatus does not copy applicationState when state field is missing", will it work?

FAUST-BENCHOU · 2026-01-07T02:07:56Z

...tomizations/sparkoperator.k8s.io/v1beta2/SparkApplication/testdata/aggregatestatus-test.yaml

+# case4. SparkApplication with missing attempts fields
+# case5. SparkApplication with missing executorState
+# case6. SparkApplication with missing lastSubmissionAttemptTime
+# case7. SparkApplication with missing terminationTime


The comments need to be updated; they don't match the test cases.

XiShanYongYe-Chang · 2026-01-07T03:17:49Z

Hi @Abhay349, just as @FAUST-BENCHOU said, some use case descriptions seem to be inconsistent with the content. Could you help revise the descriptions of the use cases? (The test cases themselves are fine.)

karmada-bot requested review from Poor12 and chaunceyjiang January 5, 2026 14:40

karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 5, 2026

gemini-code-assist bot reviewed Jan 5, 2026

View reviewed changes

Abhay349 force-pushed the sparkapplication-interpreter-tests branch 2 times, most recently from e3886af to 38eee15 Compare January 5, 2026 15:01

karmada-bot assigned XiShanYongYe-Chang Jan 6, 2026

Abhay349 force-pushed the sparkapplication-interpreter-tests branch from 38eee15 to 2f0a1b3 Compare January 6, 2026 19:56

karmada-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 6, 2026

Add complete test cases for SparkApplication

4a02960

Signed-off-by: Abhay349 <[email protected]>

Abhay349 force-pushed the sparkapplication-interpreter-tests branch from 2f0a1b3 to 4a02960 Compare January 6, 2026 19:56

XiShanYongYe-Chang reviewed Jan 7, 2026

View reviewed changes

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 7, 2026

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 7, 2026

karmada-bot merged commit 050353e into karmada-io:master Jan 7, 2026
21 checks passed

FAUST-BENCHOU reviewed Jan 7, 2026

View reviewed changes

XiShanYongYe-Chang mentioned this pull request Jan 7, 2026

[Umbrella] Enhance third-party resource interpreter tests with output verification #6952

Open

39 tasks

Abhay349 mentioned this pull request Jan 7, 2026

Add complete test cases for SparkApplication with description fixed #7091

Open

Add complete test cases for SparkApplication #7087

Add complete test cases for SparkApplication #7087

Conversation

Abhay349 commented Jan 5, 2026

What does this PR do?

Uh oh!

gemini-code-assist bot commented Jan 5, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

XiShanYongYe-Chang commented Jan 6, 2026

Uh oh!

XiShanYongYe-Chang commented Jan 6, 2026

Uh oh!

FAUST-BENCHOU commented Jan 6, 2026

Uh oh!

FAUST-BENCHOU commented Jan 6, 2026

AggregateStatus

InterpretComponent

InterpretHealth

InterpretStatus (ReflectStatus)

Uh oh!

FAUST-BENCHOU commented Jan 6, 2026

Uh oh!

Abhay349 commented Jan 6, 2026

Uh oh!

Abhay349 commented Jan 6, 2026

Uh oh!

FAUST-BENCHOU commented Jan 6, 2026

Uh oh!

Abhay349 commented Jan 6, 2026

Uh oh!

Abhay349 commented Jan 6, 2026

Uh oh!

XiShanYongYe-Chang commented Jan 7, 2026

Uh oh!

FAUST-BENCHOU commented Jan 7, 2026

Uh oh!

XiShanYongYe-Chang left a comment

Choose a reason for hiding this comment

Uh oh!

karmada-bot commented Jan 7, 2026

Uh oh!

Uh oh!

FAUST-BENCHOU Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Abhay349 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

FAUST-BENCHOU Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

FAUST-BENCHOU Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Abhay349 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

FAUST-BENCHOU Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

XiShanYongYe-Chang commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

codecov-commenter commented Jan 5, 2026 •

edited

Loading