fix: enhance test_checkers to cover new situations

[Saksham-Sirohi]

Summary

This PR introduces new test cases in test/test_checkers.py to confirm that the MyFakeChecker class can detect multiple versions of a single product in a file correctly after the changes introduced in patch #4911.

Changes Introduced

New Test Cases:

1. Test Multiple Versions Detection:

   • When the file contains version strings A and B, the returned list should contain version numbers A and B.

2. Test Ignored Versions:

   • When the file contains version strings A and B, where both A and B are ignored, the returned list should have "UNKNOWN" as its only element.

3. Test Mixed Versions:

   • When the file contains version strings A, B, and C, where B is ignored, the returned list should contain version numbers A and C.

Checklist

• Code adheres to the project’s coding standards.
• Tests executed with pytest test/test_checkers.py confirm new test cases pass correctly.
• All existing tests pass successfully.

Steps to Test

Run pytest test/test_checkers.py to verify:

• The new test cases confirm that multiple versions are detected correctly.
• Ignored versions are handled as expected.
• Mixed versions detection works as intended.

Related Issues

Fixes #4940

Additional Notes

• These new test cases ensure that the MyFakeChecker class works correctly with the new functionality introduced by patch fix: Support detecting multiple product versions #4911.
• The changes improve test coverage and ensure that multiple versions detection is validated, maintaining the integrity of the checker logic.

[stvml]

Thanks for taking a look at this issue, @22f1001635! Could you please alter the test logic as I've described, to make sure we're thoroughly testing the new functionality?

[Saksham-Sirohi]

hi @stvml , i have got rid from re.compile and have made the tests easier to understand avoiding repeated assignments
however, while making the tests I have come across this
Mixed Valid and Ignored Versions:

Expected: ["5.8", "6.0"]

Actual: ["UNKNOWN"]

This suggests that the get_versions method is not correctly handling ignored versions (5.6) and is returning ["UNKNOWN"] instead of the valid versions. unknown would have been fine as well if the other two versions also appeared in the list

I was expecting a empty list for no version pattern that came out fine

Duplicate Versions:

Expected: ["5.8", "6.0"]

Actual: ["5.8", "5.8", "6.0"]

This suggests that the get_versions method is not removing duplicate versions.This is however,not as critical as mixed versions we can however remove duplicates by just using sets instead of list

AssertionError: Mixed valid and ignored versions failed
E assert ['UNKNOWN'] == ['UNKNOWN', '5.8', '6.0']
E
E Right contains 2 more items, first extra item: '5.8'
E
E Full diff:
E [
E 'UNKNOWN',
E - '5.8',
E - '6.0',
E ]

test/test_checkers.py:182: AssertionError
Please take a look and let me know your thoughts Also I highly recommend moving towards sets as it avoids duplicate value and ordering issues aren't prevalant

[terriko]

It's not clear to me that this is ready yet, but it looks safe to run the CI so I'll kick that off now.

[stvml]

Okay, a couple of points of feedback here:

1. Fully agree with switching to a set instead of a list here. I think duplicate version entries for a single file could create unintended behavior on real-world inputs. You will have to modify VersionMatchInfo and update each place where we use that.
2. I think your mixed-versions test case has uncovered another issue, which is that in the regex_find() utility method in util.py, we should probably be passing match.group(0) to check_ignored instead of match.string. As written, we will ignore any version that occurs in the same file as an ignored version string, which is not the intention of the ignore patterns feature. This is why we write tests!
3. For that mixed-versions test case, your expected output is ['UNKNOWN', '5.8', '6.0'], which should never happen. If there are any valid versions found, we don't add UNKNOWN to the returned list of versions. UNKNOWN only happens when no version numbers are discovered.
4. Finally: TestMyFakeChecker isn't a good name for this test class. We're testing the version parser, which is why the other class is called TestCheckerVersionParser.

From this and your previous commits, my guess is that you're leaning heavily on an AI code assistant to write these patches. That's fine, those tools can be really helpful, but I think your contributions would benefit from a bit more human touch to make sure everything makes sense. LLMs are great at writing syntactically correct snippets of code, but they aren't as good yet at making changes that are consistent and readable in the context of a larger codebase.

Thanks for working on this through all the back and forth!

[stvml]

See my comments above. Tests will also have to pass, obviously :)

[Saksham-Sirohi]

Hey @stvml ,
I really appreciate your feedback! You're absolutely right that I often use an LLM to get a better understanding of what I’m working with, especially as I’m still getting familiar with this repo and being a beginner to open-source. That being said, I usually take the time to carefully review and refine the changes before committing as you can see from resolution of other issues. However, this time, since it was a "good first issue" and seemed relatively easy, I didn’t pay as much attention as I should have.

That said, in my last commit, I took matters into my own hands, found some issues, and flagged them right away. I completely understand the importance of maintaining consistency and correctness, and I assure you this won’t happen again.

To be honest, the past week has been incredibly busy for me due to a combination of exams and work, which has made it hard to dedicate as much time as I’d like. I’ve taken on several issues in the repository, but if my schedule doesn’t ease up in the next couple of days, I might drop 1-2 and hand them over to someone as my workload will be heavy for about two more weeks, but after that, I should be back to contributing more effectively.
Thanks again for your patience and guidance—I really appreciate it! And sorry for making this issue take up so much of your time.

[Saksham-Sirohi]

Hi @terriko @stvml test pass successfully for test_checkers required changes has been made please take a look

[stvml]

Just a couple of comment fixes, otherwise LGTM!

[stvml]

Understood! No worries at all about the time needed on my end, this patch needed to be written and we ultimately have the same goal of improving this open-source project.

[stvml]

Should be the last change here. A couple of test failures: the sqlite checker in sqlite.py overrides get_versions to do some custom version mapping logic. Looks like it's still using append, which fails once we made the change to a set...

[Saksham-Sirohi]

I have made the requested changes and all tests and workflows pass on the branch