Skip to content

Track tests in HasTests to support metadata needs #4833

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

Conversation

tmckay-sifive
Copy link
Contributor

@tmckay-sifive tmckay-sifive commented Mar 26, 2025

Contributor Checklist

  • Did you add Scaladoc to every public function/method?
  • Did you add at least one test demonstrating the PR?
  • Did you delete any extraneous printlns/debugging code?
  • Did you specify the type of improvement?
  • Did you add appropriate documentation in docs/src?
  • Did you request a desired merge strategy?
  • Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • API modification

Desired Merge Strategy

  • Squash: The PR will be squashed and merged (choose this if you have no preference).

Release Notes

  • Refactor HasTests to track tests in buffer before elaborating
    • The use case for this is to support capturing metadata about the tests that belong to a HasTests module. At the moment, testName is the only public member of the TestParameters, but this may be expanded in the future.

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels? (Select the most appropriate one based on the "Type of Improvement")
  • Did you mark the proper milestone (Bug fix: 3.6.x, 5.x, or 6.x depending on impact, API modification or big change: 7.0)?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you do one of the following when ready to merge:
    • Squash: You/ the contributor Enable auto-merge (squash) and clean up the commit message.
    • Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

@jackkoenig jackkoenig added the No Release Notes Exclude from release notes, consider using Internal instead label Mar 26, 2025
@jackkoenig
Copy link
Contributor

Marked as No Release Notes instead of API Modification because there is not yet a release with this feature.

@tmckay-sifive tmckay-sifive force-pushed the tmckay/inline-test-internals-refactor branch from 797f20e to 732b41f Compare April 1, 2025 18:22
Copy link
Contributor

@jackkoenig jackkoenig left a comment

Choose a reason for hiding this comment

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

I don't entirely understand why you're making this change, can you explain a little bit more in the PR description?

@tmckay-sifive
Copy link
Contributor Author

tmckay-sifive commented Apr 9, 2025

Of course, the focus here is supporting metadata, e.g. Properties. I added

The use case for this is to support capturing metadata about the tests that belong to a HasTests module. At the moment, testName is the only public member of the TestParameters, but this may be expanded in the future.

to the description and a test showing how this might be useful.

@tmckay-sifive tmckay-sifive changed the title Track tests in chisel3.experimental.inlinetest.HasTests Track tests in HasTests to support metadata use-cases Apr 9, 2025
@tmckay-sifive tmckay-sifive changed the title Track tests in HasTests to support metadata use-cases Track tests in HasTests to support metadata needs Apr 9, 2025
@jackkoenig jackkoenig added this to the 7.0 milestone Apr 15, 2025
@tmckay-sifive tmckay-sifive merged commit 1adb02b into chipsalliance:main Apr 15, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Release Notes Exclude from release notes, consider using Internal instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants