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 heuristics about which file to update in DependencyManager #180

Merged
merged 4 commits into from
Dec 19, 2023

Conversation

andrecsilva
Copy link
Contributor

  • Also added a report listing if dependencies were added successfully or not.
  • Added some robustness to RequirementsTxtWriter (this is mostly a band-aid and needs proper, more general, handling).

Copy link

codecov bot commented Dec 14, 2023

Codecov Report

Merging #180 (5cabfb1) into main (966bf5e) will decrease coverage by 0.24%.
The diff coverage is 82.22%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #180      +/-   ##
==========================================
- Coverage   96.42%   96.19%   -0.24%     
==========================================
  Files          79       79              
  Lines        3723     3754      +31     
==========================================
+ Hits         3590     3611      +21     
- Misses        133      143      +10     
Files Coverage Δ
...codemodder/project_analysis/python_repo_manager.py 87.50% <ø> (-8.34%) ⬇️
src/codemodder/context.py 97.87% <94.11%> (+0.22%) ⬆️
src/codemodder/codemodder.py 95.94% <86.66%> (-1.07%) ⬇️
...r/dependency_management/requirements_txt_writer.py 84.37% <61.53%> (-15.63%) ⬇️

... and 1 file with indirect coverage changes

@andrecsilva andrecsilva force-pushed the repo-mngr-comb branch 2 times, most recently from 47c3b21 to eed2df9 Compare December 15, 2023 12:11
@andrecsilva andrecsilva marked this pull request as ready for review December 15, 2023 12:19
Copy link
Member

@drdavella drdavella left a comment

Choose a reason for hiding this comment

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

Apologies for the delayed review. It looks good overall but I think we might need some more tests to enumerate a few more cases.

from integration_tests.base_test import SAMPLES_DIR, CleanRepoMixin


class TestDependencyManager(CleanRepoMixin):
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we have test cases here for updating pyproject.toml and setup.py as well? In those cases we could just add some simple content to those temporary files within individual test cases.

for package_store in store_list:
dm = DependencyManager(package_store, self.directory)
if (changeset := dm.write(list(dependencies), self.dry_run)) is not None:
self.add_results(codemod_id, [changeset])
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a debug log here saying which file is being written?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This information is being store in the record variable. The debug message with the file the dependency has been written in is in the new record_dependency_update function at codemodder.py

Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

2 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@andrecsilva andrecsilva added this pull request to the merge queue Dec 19, 2023
Merged via the queue into main with commit 9423b06 Dec 19, 2023
13 of 14 checks passed
@andrecsilva andrecsilva deleted the repo-mngr-comb branch December 19, 2023 11:20
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