Fix LinearDistanceAndInterpolationProvider vs removed joint

[agarwal-abhinav]

Adding a test case and changing for loop to fix the issue documented here #21860.

Closes #21860.

---

This change is 

[jwnimmer-tri]

This seems like a bug that was missed during #21397 review, so +@joemasterjohn for feature review or delegation, please.

[joemasterjohn]

Got it. Curious that this loop didn't get tickled by the hack to offset all joint indices from #21397? But either way, looks like the correct coverage is added here.

+@xuchenhan-tri for platform review tomorrow, please.

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee xuchenhan-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @agarwal-abhinav)

---

planning/test/linear_distance_and_interpolation_provider_test.cc line 458 at r1 (raw file):

  auto& plant = builder->plant();

  JointIndex joint_index(

nit: missing const

Suggestion:

  const JointIndex joint_index(

---

planning/test/linear_distance_and_interpolation_provider_test.cc line 462 at r1 (raw file):

  const Joint<double>& joint = plant.get_joint(joint_index);

  JointActuatorIndex joint_actuator_index(

nit: missing const

Suggestion:

const JointActuatorIndex joint_actuator_index(

---

planning/test/linear_distance_and_interpolation_provider_test.cc line 471 at r1 (raw file):

  const auto model = builder->Build();

  const LinearDistanceAndInterpolationProvider provider(model->plant());

With this test serving as a regression test for changes made in #21397, this is the actual line that would fail. For that reason, it makes more sense to me to spell this out surrounded by gtest predicates. It articulates the bug a bit better, and without it the unit test will just fail due to a c++ exception (rather than an gtest assertion failing). I might be wrong about this, so perhaps @jwnimmer-tri or @xuchenhan-tri could weigh in.

Suggestion:

  std::unique_ptr<LinearDistanceAndInterpolationProvider> provider;

  EXPECT_NO_THROW(provider =
                      std::make_unique<LinearDistanceAndInterpolationProvider>(
                          model->plant()));

[xuchenhan-tri]

platform

Reviewable status: 3 unresolved discussions, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @agarwal-abhinav)

---

planning/test/linear_distance_and_interpolation_provider_test.cc line 471 at r1 (raw file):

Previously, joemasterjohn (Joe Masterjohn) wrote…

With this test serving as a regression test for changes made in #21397, this is the actual line that would fail. For that reason, it makes more sense to me to spell this out surrounded by gtest predicates. It articulates the bug a bit better, and without it the unit test will just fail due to a c++ exception (rather than an gtest assertion failing). I might be wrong about this, so perhaps @jwnimmer-tri or @xuchenhan-tri could weigh in.

If the intention is the confirm that the constructor doesn't throw, then wrapping the constructor under EXPECT_NO_THROW is good enough.

EXPECT_NO_THROW(LinearDistanceAndInterpolationProvider(model->plant())); 

[xuchenhan-tri]

Reviewed 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 3 unresolved discussions, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @agarwal-abhinav)

[joemasterjohn]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: 1 unresolved discussion, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @agarwal-abhinav)

---

planning/test/linear_distance_and_interpolation_provider_test.cc line 471 at r1 (raw file):

Previously, xuchenhan-tri wrote…

If the intention is the confirm that the constructor doesn't throw, then wrapping the constructor under EXPECT_NO_THROW is good enough.

EXPECT_NO_THROW(LinearDistanceAndInterpolationProvider(model->plant())); 

Hi @agarwal-abhinav, just wanted to check in on this PR, see if you needed any clarifications from us? Should just require adding the one line that @xuchenhan-tri provided above. I'm marking myself neutral on this so my comment won't block this getting merged.

[xuchenhan-tri]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @agarwal-abhinav)

[xuchenhan-tri]

+(status: squashing now)

Reviewable status: complete! all discussions resolved, LGTM from assignees joemasterjohn,xuchenhan-tri(platform)