Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added option to get resolved packages in a requirements.txt file. Fixes #135 #160

Closed
wants to merge 2 commits into from

Conversation

arijitde92
Copy link
Contributor

Created a new option --resolved-output that finds resolved packages and writes those into the given filename in a typical requirements.txt file format. Also added test test_resolved_cli in test_cli.py.

@arijitde92
Copy link
Contributor Author

I was having an issue rebasing in the previous PR #156 . Hence opened this new PR. It is working as expected. Please check.

@TG1999
Copy link
Contributor

TG1999 commented Nov 30, 2023

@arijitde92 tests are failing in CI, please check. Thanks!

@arijitde92
Copy link
Contributor Author

Hi @TG1999 ,

After inspecting the cause of test failure, I found that a few test cases are failing but those test cases are not written by me. Please see screenshots below-
macos13
ubuntu22
win2022

As you can see, the same tests are failing for all the OSes and none of them are written by me. Is this happening with other PRs too? Or is this happening somehow because of the code I wrote? As I am not able to find any relation between the code I wrote and the tests that are failing.

Please guide me on this.
Thanks.

@TG1999
Copy link
Contributor

TG1999 commented Dec 1, 2023

@arijitde92 please take a pull from latest main branch, main branch is green :)

@arijitde92
Copy link
Contributor Author

Hi @TG1999 , I have pulled the latest main branch in my forked repo as you can see the latest commit in the screenshot below.
image

But still 1 test is failing in CI as shown in below screenshot.
image

@TG1999
Copy link
Contributor

TG1999 commented Dec 1, 2023

You have to run 'make valid' command to fix formatting errors

…#135

Created a new option --resolved-output that finds resolved packages and writes those into the given filename in a typical requirements.txt file format. Also added test test_resolved_cli in test_cli.py

Signed-off-by: Arijit De <[email protected]>
@arijitde92
Copy link
Contributor Author

Hi @TG1999 , checks are now passing. Let me know if anything else is required.

Copy link
Contributor

@TG1999 TG1999 left a comment

Choose a reason for hiding this comment

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

@arijitde92 thanks++, some nits for your consideration

@@ -54,6 +54,20 @@ def write_output_in_file(output, location):
return output


def write_resolved_packages(package_list, requirements_file):
Copy link
Contributor

Choose a reason for hiding this comment

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

@arijitde92 please add a separate unit test for this function, separated from CLI logic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TG1999 , I have written an unit test in test_cli.py (See here). Do I need to write another unit test elsewhere (maybe in test_utils.py?)?
Also could you please clarify what "separated from CLI logic" means?
I assume that you mean to test for a valid package_list. In that case do I need to create a dummy package_list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @TG1999 , could you please help in clarifying what kind of unit test you need? And in which while should I write the test?

Copy link
Contributor

Choose a reason for hiding this comment

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

@arijitde92 yes, please write a test in test_utils.py

Copy link
Contributor Author

@arijitde92 arijitde92 Jan 5, 2024

Choose a reason for hiding this comment

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

I have written two unit tests in test_utils.py. Please check.

"""
Write the resolved package names and versions into ``requirements_file_path``
"""
dependencies = package_list[0]["package_data"][0]["dependencies"]
Copy link
Contributor

Choose a reason for hiding this comment

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

what if package_list is empty? and package_data does not exist there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have implemented the checks for empty or corrupt package_list.

src/python_inspector/utils.py Outdated Show resolved Hide resolved
src/python_inspector/utils.py Outdated Show resolved Hide resolved
src/python_inspector/utils.py Outdated Show resolved Hide resolved
…#135

Added unit tests for the function write_resolved_packages in test_utils.py

Signed-off-by: Arijit De <[email protected]>
@arijitde92
Copy link
Contributor Author

Hi @TG1999 , I have added unit tests as requested and also edited the code as requested.
But now some tests are failing again. Please check in the below screenshot-
image

@arijitde92 arijitde92 closed this by deleting the head repository Oct 25, 2024
@pombredanne
Copy link
Member

@arijitde92 too bad you deleted your repo!

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