Skip to content

Conversation

@yaelbh
Copy link
Collaborator

@yaelbh yaelbh commented Dec 4, 2025

Summary

Part of #2479.
Tests for the function in the file qiskit_ibm_runtime/noise_learner_v3/validation.

Details and comments

@yaelbh yaelbh changed the base branch from main to executor_preview December 4, 2025 15:40
@yaelbh yaelbh marked this pull request as draft December 4, 2025 15:41
@yaelbh yaelbh changed the title [WIP] Tests for validation of NLV3 Tests for validation of NLV3 Dec 7, 2025
@yaelbh yaelbh marked this pull request as ready for review December 7, 2025 09:49
@yaelbh
Copy link
Collaborator Author

yaelbh commented Dec 7, 2025

Ready for review. Note that the PR contains a bug fix.

@yaelbh yaelbh changed the title Tests for validation of NLV3 [executor-preview] Tests for validation of NLV3 Dec 7, 2025
@yaelbh yaelbh added the executor-preview Issues related to the `executor_preview` branch label Dec 7, 2025
@yaelbh yaelbh requested a review from SamFerracin December 7, 2025 11:23
Copy link
Collaborator

@SamFerracin SamFerracin left a comment

Choose a reason for hiding this comment

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

Thanks! A couple of small comments

Comment on lines 33 to 45
configuration = BackendConfiguration(
backend_name="im_a_backend",
backend_version="0.0",
n_qubits=1e100,
basis_gates=["rx"],
gates=[],
local=False,
simulator=False,
conditional=True,
open_pulse=False,
memory=True,
coupling_map=[],
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not passing FakeAlgiers.configuration()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

with self.assertRaisesRegex(ValueError, "xslow"):
validate_options(options=options, configuration=configuration)

def test_validate_instruction(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a big test function with many assert. In general, I prefer breaking big test functions into many smaller test functions with asserts targeting a single concern, for example:

  • test_validate_instruction_passes_when_options_valid
  • test_validate_instruction_without_twirl_raises
  • test_validate_instruction_without_box_raises
  • ...

The benefit is that if we make a change that invalidate one of these asserts, we immediately get a hint of what caused the break by reading the name(s) of the broken tests, as opposed to having to dig through

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please check the solution of using subTest (2340427).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

executor-preview Issues related to the `executor_preview` branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants