You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In my initial review, i raised the concern that the new SharedPaths feature wasn't being sufficiently tested because there only seemed to be a happy path test and no sad paths. In the ensuing discussion, we hit on the following points which Rui asked me to write down to jog our memories later:
that this new feature hooked into a shared modular unit, ie "lookupPaths",
that "lookupPaths" was being implemented and tested sufficiently by itself,
that the test in question in the PR was just to make sure it hooked up into the shared modular unit correctly,
my stance that we should explicitly have a unit test in ssm_test.go to make sure that it hooks into the shared modular unit, and also that we could just rely on the shared modular unit to do all the happy and sad path testing for us
that Rui pointed out all the other credential managers have this exact single happy path integration test, and that the PR author probably just copied it from there
Rui's worry that perhaps there actually wasn't sufficient test coverage on the shared modular unit at all.
As such, Rui asked me to write this issue so we could investigate later whether or not the shared modular unit (lookupPaths) actually does have sufficient testing. Rui did a quick search with me and couldn't actually find tests for it, so we should revisit this later and make sure it's being tested properly.
Also tagging @xtremerui just so this pops up in your feed, even though you're watching me type this lol.
The text was updated successfully, but these errors were encountered:
Summary
Rui and i were discussing this PR he asked me to review, and specifically this test here in the ssm_test.go.
In my initial review, i raised the concern that the new SharedPaths feature wasn't being sufficiently tested because there only seemed to be a happy path test and no sad paths. In the ensuing discussion, we hit on the following points which Rui asked me to write down to jog our memories later:
As such, Rui asked me to write this issue so we could investigate later whether or not the shared modular unit (lookupPaths) actually does have sufficient testing. Rui did a quick search with me and couldn't actually find tests for it, so we should revisit this later and make sure it's being tested properly.
Also tagging @xtremerui just so this pops up in your feed, even though you're watching me type this lol.
The text was updated successfully, but these errors were encountered: