Skip to content
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

Fix bug found by WriteIndirectMeterEntryTest test case #316

Merged
merged 3 commits into from
Nov 12, 2024

Conversation

ffoulkes
Copy link
Collaborator

@ffoulkes ffoulkes commented Nov 8, 2024

  • The WriteIndirectMeterEntryTest test case failed because WriteMeterEntry() was calling FindResourceTypeByID() with the ID returned by GetTdiRtId(), not a P4 resource ID. Changed it to look up meter_entry.meter_id() instead.

  • Also renamed the variable to which the value returned by GetTdiRtId() is assigned, from meter_id to meter_rt_id, in hopes of reducing confusion in the future.

  • Reenabled the test case.

See issue #306 for a description of the problem.

- The WriteIndirectMeterEntryTest test case failed because
  WriteMeterEntry() was calling FindResourceTypeByID() with the
  meter ID returned by GetTdiRtId(), which is not defined in
  P4Info. Changed it to look up meter_entry.meter_id() instead.

  Also renamed the variable to which the value returned by
  GetTdiRtId() is assigned, from meter_id to meter_rt_id,
  in hopes of reducing confusion in the future.

Signed-off-by: Derek Foster <derek.foster@intel.com>
Signed-off-by: Derek Foster <derek.foster@intel.com>
@ffoulkes ffoulkes linked an issue Nov 8, 2024 that may be closed by this pull request
- Created a separate GitHub issues for each of the failing test
  cases, so they can be tracked individually. Updated the comments
  in tdi_table_manager_test_case.cc to refer to the correct issue.

Signed-off-by: Derek Foster <derek.foster@intel.com>
@ffoulkes ffoulkes linked an issue Nov 8, 2024 that may be closed by this pull request
@ffoulkes ffoulkes changed the title Fix WriteIndirectMeterEntryTest test case Fix bug found by WriteIndirectMeterEntryTest test case Nov 8, 2024
@ffoulkes ffoulkes linked an issue Nov 9, 2024 that may be closed by this pull request
@ffoulkes ffoulkes added tests Affects unit tests bug Something isn't working labels Nov 12, 2024
Copy link
Collaborator

@5abeel 5abeel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@5abeel 5abeel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ffoulkes ffoulkes merged commit e7c1afa into split-arch Nov 12, 2024
5 checks passed
@ffoulkes ffoulkes deleted the write-meter-entry branch November 12, 2024 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tests Affects unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WriteIndirectMeterEntryTest test case is failing
2 participants