Skip to content

Conversation

inosmeet
Copy link
Contributor

added test to check loader's functionality.

cc @terriko @anthonyharrison

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

The windows tests are failing on this test particularly, which makes me think that maybe it's not dealing with the path correctly or something. I'm re-running the tests just in case it's a random fluke but I suspect not so I'm marking this as needing fixes.

Not sure what the fix would be but I'd suggest try using pathlib (e.g. the Path library) as we do elsewhere in cve-bin-tool instead of os.path and see if that helps? One of the major reasons we use it is that it seamlessly solves some windows/linux discrepancies for us.

If you don't have a windows box set up for testing locally, don't forget that you can set up github actions on your fork for messing around if you need to so you can have something that runs just this test and doesn't need to install all the stuff to run the pdf tests (which are part of why windows tests take so much longer than the linux ones)

Signed-off-by: Meet Soni <[email protected]>
Signed-off-by: Meet Soni <[email protected]>
Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Thanks! Looks like it's behaving now so I'll get it merged.

@terriko terriko merged commit f6f6e57 into intel:main Jul 18, 2024
@inosmeet inosmeet deleted the test-script branch July 19, 2024 03:50
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.

2 participants