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

Updated match.py to handle matched_text_diagnostics seperately #3510

Closed
wants to merge 1 commit into from

Conversation

tehami02
Copy link
Contributor

@tehami02 tehami02 commented Sep 14, 2023

Updated match.py to handle matched_text_diagnostics seperately #3426

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 📑 and links the original issue above 🔗
  • Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 📁
  • Looked for possible updates in documentation and added updates if applicable
  • Updated CHANGELOG.rst

Signed-off-by: Mohd Tehami [email protected]

@tehami02
Copy link
Contributor Author

tehami02 commented Sep 14, 2023

Some tests are failing and internally they are failing due to tests/scancode/test_cli.py , even locally this test fails and shows - src/scancode/cli_test_utils.py: 210: AssertionError that is - On line 210 I found this - https://github.com/nexB/scancode-toolkit/blob/e005a91c649cdf52b340882e072043008911562e/src/scancode/cli_test_utils.py#L207C2-L207C2
Probably the results does not contain expected data.

    if results != expected:
    expected = saneyaml.dump(expected)
    results = saneyaml.dump(results)
    assert results == expected

Does this means my logic of code is wrong or can regenerating this test fix the failure ?

@tehami02 tehami02 changed the title Updated match.py to handle matched_text_diagnostics seperately #3450 Updated match.py to handle matched_text_diagnostics seperately Sep 14, 2023
@pombredanne
Copy link
Member

@tehami02 Thanks! any plan to push this through?

@tehami02
Copy link
Contributor Author

tehami02 commented Oct 12, 2023

@pombredanne Yeah, but I think I'll need to pass all the tests. If you have any solution about how can I make my code pass tests that are failing please share.

@AyanSinhaMahapatra
Copy link
Member

@tehami02 thanks++ for the PR and sorry for the late reply here, this fell through the cracks 😅

For future reference, see https://scancode-toolkit.readthedocs.io/en/stable/contribute/contrib_dev.html#running-tests for more info on running tests and looking into failures to check whether the code is working as intended and if you need to regenerate test fixtures by using something like: SCANCODE_REGEN_TEST_FIXTURES=yes pytest -vvs tests/<path_to_test_file_or_function> --lf

Here there were a few more changes required to just add the new attribute, see 2ddb31c#diff-e6d56ce33d4c6e9fcf1d10ee56286e5e11f5eb07739ca062b401e798a684f2c7R789, and then a lot more to make sure the license detection objects in packages/top-level unique detections also reflect with a new attribute in a same way, and other plugins work as intended without failing, see 2ddb31c for reference. I had a lot more changes in license detection also added, so to avoid conflicts I'm adding your change in that PR instead.

I've used your commit here at #3620 and then added my changes on top of it, so you see what more you should have done here. I'm closing this PR as I've already used the code from this PR. Thanks again, and you can work on other good first issues instead: https://github.com/nexB/scancode-toolkit/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@tehami02
Copy link
Contributor Author

tehami02 commented Dec 20, 2023

@AyanSinhaMahapatra Thanks for completing the issue. I'll go through your PR to understand how things work here. Btw can u suggest anything else I should do to understand working of this software and codebase more efficiently as I would like to contribute more into it with greater understanding as I can see many issues are unresolved.

@AyanSinhaMahapatra
Copy link
Member

@tehami02 see https://github.com/nexB/scancode-toolkit/issues?q=is%3Aopen+is%3Aissue+label%3Apackage-formats for supporting more package formats in SCTK, there are nice to look into and add support for.

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