Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
🌱 add dependency package in test/api #375
🌱 add dependency package in test/api #375
Changes from 9 commits
6ed6cd0
06e448e
a8449a5
a07ee95
5ad6f23
3f4253c
08ad217
cb13080
890a794
16efa1c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a Dependency creation and condition below should check if
got
is an array with the created dependency. Sorry, I missed this part before.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall I just compare the ID's of dependency created and the ApplicationFrom/To ID's will it suffice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found out we're using the assert.FlatEqual method in a wrong way (also in some other tests outside of this PR), the condition should look like
if !assert.FlatEqual(got, Samples) { //error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was wondering too when working on the part before! This might be a breaking change in the code, as inverting the condition will make the test fail. Will dig deep on the same .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thihs would delete all dependencies in API, not only ones created by this test. Not saying it is blocker in this case, but better track created objects to be deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya that's something should be taken care of. A way to tackle this what I can think of is creating an array of ID's and store ID's of dependencies at their time of creation. The Delete() of the Dependency will work on the array of ID's and will only delete them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we have (or we will) ensure that all the created Dependencies are present in the Dependency.List(), we will ensure that all the dependencies are used and thus they are allowed to be deleted.