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

Fix failing tests in the python ecosystem #10386

Merged
merged 9 commits into from
Aug 12, 2024
Merged

Conversation

amazimbe
Copy link
Contributor

@amazimbe amazimbe commented Aug 7, 2024

What are you trying to accomplish?

3 tests are failing on CI in the python ecosystem.

Failed examples:

rspec ./spec/dependabot/python/file_parser/setup_file_parser_spec.rb:178 # Dependabot::Python::FileParser::SetupFileParser for setup.cfg parse length is expected to eq 15
rspec ./spec/dependabot/python/file_parser/setup_file_parser_spec.rb:219 # Dependabot::Python::FileParser::SetupFileParser for setup.cfg parse a tests_require dependencies has the right details
rspec ./spec/dependabot/python/file_parser_spec.rb:999 # Dependabot::Python::FileParser parse with a setup.cfg length is expected to eq 15

It looks like we are running a python script in a subprocess to parse a setup.cfg file here:
https://github.com/dependabot/dependabot-core/blob/main/python/helpers/run.py#L13 .

I'm not sure if something has changed in the python parser but for some reason it's
not including the dependencies under tests_require and that's causing some assertions
in the tests to fail.

The dependencies missing / being ignored are here:
https://github.com/dependabot/dependabot-core/blob/main/python/spec/fixtures/setup_files/setup_with_requires.cfg#L25

#10388

How will you know you've accomplished your goal?

Tests will be green again

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@amazimbe amazimbe force-pushed the amazimbe/fix-pip-tests branch 2 times, most recently from 6d0ccb2 to d1732fe Compare August 8, 2024 09:41
@amazimbe amazimbe changed the title [WIP] Test to see if this is a regression Update failing tests in the python ecosystem Aug 8, 2024
@amazimbe amazimbe marked this pull request as ready for review August 8, 2024 09:55
@amazimbe amazimbe requested a review from a team as a code owner August 8, 2024 09:55
Copy link
Member

@jurre jurre left a comment

Choose a reason for hiding this comment

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

It looks like this just updates the tests to reflect that those dependencies aren't being included?

Let's instead figure out what caused this to change, determine if it is a regression and fix it if so.

@amazimbe
Copy link
Contributor Author

amazimbe commented Aug 8, 2024

It looks like this just updates the tests to reflect that those dependencies aren't being included?

Let's instead figure out what caused this to change, determine if it is a regression and fix it if so.

I agree though my concern was that we'll be blocked from merging and shipping.

@honeyankit
Copy link
Contributor

./spec/dependabot/python/file_parser/setup_file_parser_spec.rb:178

@amazimbe : The issue is that the two dependencies (pytest==2.9.1, responses==0.5.1) are not present in the parsed output after the method parse_setup in the native helper (parser.py) is executed.

The code takes all the dependencies from the fixture setup_with_requires.cfg which is 15 and write to temp setup.cfg which is correct but when the native helper parse the temp setup.cfg at this place there only 13 dependencies and two of the above stated dependencies is missing.

@honeyankit
Copy link
Contributor

./spec/dependabot/python/file_parser/setup_file_parser_spec.rb:178

@amazimbe : The issue is that the two dependencies (pytest==2.9.1, responses==0.5.1) are not present in the parsed output after the method parse_setup in the native helper (parser.py) is executed.

The code takes all the dependencies from the fixture setup_with_requires.cfg which is 15 and write to temp setup.cfg which is correct but when the native helper parse the temp setup.cfg at this place there only 13 dependencies and two of the above stated dependencies is missing.

@amazimbe I should had seen your initial description. You already got this.

@amazimbe amazimbe changed the title Update failing tests in the python ecosystem Fix failing tests in the python ecosystem Aug 12, 2024
Copy link
Member

@jurre jurre left a comment

Choose a reason for hiding this comment

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

Thanks @honeyankit for following up, that seems to address the underlying issue 🙇

@amazimbe amazimbe merged commit 2691f1b into main Aug 12, 2024
67 checks passed
@amazimbe amazimbe deleted the amazimbe/fix-pip-tests branch August 12, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants