Skip to content

fix: Correct typo 'occured' to 'occurred'#5950

Open
thecaptain789 wants to merge 2 commits intofeast-dev:masterfrom
thecaptain789:fix/typo-occurred
Open

fix: Correct typo 'occured' to 'occurred'#5950
thecaptain789 wants to merge 2 commits intofeast-dev:masterfrom
thecaptain789:fix/typo-occurred

Conversation

@thecaptain789
Copy link

@thecaptain789 thecaptain789 commented Feb 7, 2026

Fixed typo in variable name in dependency conflict test.


Open with Devin

@thecaptain789 thecaptain789 requested a review from a team as a code owner February 7, 2026 16:08
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

Comment on lines +31 to +32
conflict_occurred = "dependency conflicts" in err and "ERROR" not in err
assert not conflict_occurred, "Dependency conflict detected during installation"
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 Dependency conflict check condition is logically inverted, making the test ineffective

The condition on line 31 uses and to combine two checks, but this means actual pip dependency conflicts are never caught.

Root Cause

When pip detects dependency conflicts but still succeeds (exit code 0), it outputs to stderr a message like:

ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. Running pip check may reveal dependency conflicts.

The current condition is:

conflict_occurred = "dependency conflicts" in err and "ERROR" not in err

Since pip's conflict message contains BOTH "dependency conflicts" AND "ERROR", the evaluation is:

  • "dependency conflicts" in errTrue
  • "ERROR" not in errFalse
  • True and FalseFalse

So conflict_occurred is False, and assert not False passes. The test will never detect actual dependency conflicts from pip.

The condition should likely be:

conflict_occurred = "dependency conflicts" in err or "ERROR" in err

or simply:

conflict_occurred = "dependency conflicts" in err

Impact: The test is meant to catch dependency conflicts between Feast and KServe, but it will always pass regardless of whether conflicts exist, defeating its purpose.

Suggested change
conflict_occurred = "dependency conflicts" in err and "ERROR" not in err
assert not conflict_occurred, "Dependency conflict detected during installation"
conflict_occurred = "dependency conflicts" in err
assert not conflict_occurred, "Dependency conflict detected during installation"
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Collaborator

@shuchu shuchu left a comment

Choose a reason for hiding this comment

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

lgtm

@ntkathole ntkathole changed the title fix: correct typo 'occured' to 'occurred' fix: Correct typo 'occured' to 'occurred' Feb 11, 2026
@ntkathole
Copy link
Member

@thecaptain789 Please sign the commit, you can hit UI button to pass DCO

Copy link
Collaborator

@shuchu shuchu left a comment

Choose a reason for hiding this comment

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

please sign off your commit.

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.

3 participants