Skip to content

Conversation

@dsteeley
Copy link
Contributor

We have hit issues with the PoetryDetector.

[ERROR] SafelyExecuteAsync logged TomlException: (991,2) : error : Unable to set the property extras on object type Microsoft.ComponentDetection.Detectors.Poetry.Contracts.PoetryLock.

I believe this is due to an issue with Tomlyn where defaulting missing tables didn't work for object types. This has now been fixed in version 0.15.1.
Corresponding PR

Could I please request a release of component-detection once this change is in, to fixup my pipelines?

Question for reviewer:

  • Why did the testing in component-detection not identify this issue and could you suggest some additional testing to add to verify that PRs haven't broken detectors.

@dsteeley dsteeley requested a review from a team as a code owner October 17, 2022 15:48
@dsteeley dsteeley requested a review from amitla1 October 17, 2022 15:48
@JamieMagee
Copy link
Member

Thanks for the PR.

Do you have a sample poetry.lock we can use in a unit test to make sure we don't regress this behaviour in future?

@dsteeley
Copy link
Contributor Author

The following test has a few example poetry.lock files that I would have expected to catch this.
https://github.com/microsoft/component-detection/blob/main/test/Microsoft.ComponentDetection.Detectors.Tests/PoetryComponentDetectorTests.cs#L88

I can provide a full example file if there is another location that tests are run?

@JamieMagee
Copy link
Member

In the example you shared we're missing metadata object? Have I got that right?

@cobya cobya added type:bug Bug fix of existing functionality detector:poetry The poetry detector labels Oct 17, 2022
@dsteeley
Copy link
Contributor Author

Yes the metadata object isn't tested and example would be the toml table below

[metadata]
lock-version = "1.1"
python-versions = "^3.8"
content-hash = "af93ed08cae8fee3abe9fefd73c2471b3c195dadc4ce1f316ad68bf68ed170dc"

I think that the tests weren't being run somehow as the minimal example would have hit the issue of being unable to generate the extras Dictionary from default values.

@JamieMagee
Copy link
Member

Okay, I'll merge this and cut a release to unblock you. I created #304 to followup on why this was not caught by our unit tests.

@JamieMagee JamieMagee enabled auto-merge (squash) October 17, 2022 21:17
@JamieMagee JamieMagee merged commit 4e1f8c0 into microsoft:main Oct 17, 2022
@github-actions
Copy link

👋 Hi! It looks like you modified some files in the Detectors folder.
You may need to bump the detector versions if any of the following scenarios apply:

  • The detector detects more or fewer components than before
  • The detector generates different parent/child graph relationships than before
  • The detector generates different devDependencies values than before

If none of the above scenarios apply, feel free to ignore this comment 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

detector:poetry The poetry detector type:bug Bug fix of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants