add data-test-id for console dashboard components

[dantrainor]

No description provided.

[openshift-ci-robot]

Hi @dantrainor. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dantrainor
To complete the pull request process, please assign rhamilto
You can assign the PR to them by writing /assign @rhamilto in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• frontend/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[dantrainor]

/assign @rhamilto

[spadgett]

Please add the attributes in the same PR as the tests so we can see how they will be used.

[honza]

Please add the attributes in the same PR as the tests so we can see how they will be used.

They are in this PR. Only the metal3 ones are elsewhere to ease the review burden.

[dantrainor]

Also added data-test-id attributes for the counts of such Items

[spadgett]

They are in this PR. Only the metal3 ones are elsewhere to ease the review burden.

I'm confused... These are added for specific integration tests, correct? There aren't any tests in this PR. It's difficult to evaluate if the changes make sense without seeing how they're used.

[dantrainor]

They are in this PR. Only the metal3 ones are elsewhere to ease the review burden.

I'm confused... These are added for specific integration tests, correct? There aren't any tests in this PR. It's difficult to evaluate if the changes make sense without seeing how they're used.

They are to facilitate the integration tests as part of #1886, which were originally written using brittle xpaths. These changes and the ones in #2288 are used for more precise identification of the elements needed to test against. This PR was split from #2288 to try and get the changes reviewed and accepted quicker, with the idea of having a different set of reviewers able to review #2288 quicker.

[spadgett]

Right, understood. But it's impossible to review this change in isolation, and it should only be merged when the tests are merged. IMO, breaking it out actually makes reviews harder.

[dantrainor]

Right, understood. But it's impossible to review this change in isolation, and it should only be merged when the tests are merged. IMO, breaking it out actually makes reviews harder.

Understood, thanks for the feedback.

Can you provide some guidance on how to proceed with these PRs as they relate to each other? Should #2287 and #2288 become part of #1886?

[spadgett]

Yes, I would roll both changes into #1886

[dantrainor]

Yes, I would roll both changes into #1886

I appreciate the help thank you.

[dantrainor]

This is going to become part of #1886