Skip to content

Conversation

vishal-chdhry
Copy link
Member

@vishal-chdhry vishal-chdhry commented Sep 4, 2023

Explanation

Closes: #8008

if err := d.loadData(l, idx); err != nil {
	return nil
}

I believe this should not return nil, as the value of err will be lost.

Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Merging #8234 (d27b423) into main (477f1a0) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #8234      +/-   ##
==========================================
+ Coverage   33.85%   33.89%   +0.04%     
==========================================
  Files         260      260              
  Lines       23570    23570              
==========================================
+ Hits         7980     7990      +10     
+ Misses      14766    14759       -7     
+ Partials      824      821       -3     
Files Changed Coverage Δ
pkg/engine/context/deferred.go 86.66% <100.00%> (+9.52%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@JimBugwadia
Copy link
Member

Good catch, @vishal-chdhry!

JimBugwadia
JimBugwadia previously approved these changes Sep 4, 2023
@eddycharly eddycharly enabled auto-merge (squash) September 4, 2023 07:46
eddycharly
eddycharly previously approved these changes Sep 4, 2023
Copy link
Member

@eddycharly eddycharly left a comment

Choose a reason for hiding this comment

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

Good catch !

@eddycharly
Copy link
Member

BTW, it would be nice to have a test for that. This is not something we want to have regressions.

eddycharly and others added 2 commits September 4, 2023 12:30
this loader simulates an invalid loader, such as invalid API call URL or a bad request to external service

Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
@vishal-chdhry vishal-chdhry dismissed stale reviews from eddycharly and JimBugwadia via 74b0bf4 September 4, 2023 12:20
@vishal-chdhry
Copy link
Member Author

BTW, it would be nice to have a test for that. This is not something we want to have regressions.

@eddycharly Something like this: 74b0bf4 (#8234)? This loader simulates an invalid loader, such as invalid API call URL or a bad request to external service

Copy link
Member

@eddycharly eddycharly left a comment

Choose a reason for hiding this comment

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

Thanks for adding the test !

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.

[Bug] Different behaviours for different values of --enableDeferredLoading
3 participants