Skip to content

Comments

test/orfs/mock-array: test IO_CONSTRAINTS and MACRO_PLACEMENT_TCL#8273

Closed
oharboe wants to merge 1 commit intoThe-OpenROAD-Project:masterfrom
Pinata-Consulting:test-orfs-pin-macro-placement
Closed

test/orfs/mock-array: test IO_CONSTRAINTS and MACRO_PLACEMENT_TCL#8273
oharboe wants to merge 1 commit intoThe-OpenROAD-Project:masterfrom
Pinata-Consulting:test-orfs-pin-macro-placement

Conversation

@oharboe
Copy link
Collaborator

@oharboe oharboe commented Sep 9, 2025

No description provided.

@oharboe oharboe requested a review from maliberty September 9, 2025 03:25
@oharboe
Copy link
Collaborator Author

oharboe commented Sep 9, 2025

@maliberty This test write IO/macro placement + use of these.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2025

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe oharboe force-pushed the test-orfs-pin-macro-placement branch from 995a3a4 to aafb95d Compare September 9, 2025 03:34
@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2025

clang-tidy review says "All clean, LGTM! 👍"

@oharboe
Copy link
Collaborator Author

oharboe commented Sep 9, 2025

@maliberty unrelated CI failure on pr-merge

image

@maliberty
Copy link
Member

Why do these need to be part of mock-array and not an ordinary unit test that would run much faster. What additional coverage does this provide?

@oharboe
Copy link
Collaborator Author

oharboe commented Sep 9, 2025

Why do these need to be part of mock-array and not an ordinary unit test that would run much faster. What additional coverage does this provide?

Easy to articulate this way and it tests reading as well as writing of these files for routing by abutment and IO pins on multiple layers.

@maliberty
Copy link
Member

| Easy to articulate this way and it tests reading as well as writing of these files for routing by abutment and IO pins on multiple layers.

All those things can be unit tested. This adds a lot of runtime overhead. I view flow tests as testing the interaction across a full flow not testing the functionality of a single module.

@oharboe
Copy link
Collaborator Author

oharboe commented Sep 9, 2025

Tests also need to be cheap to articulate.

Do you already have this coverage in unit tests?

I think runtime concerns/cost of developing tests are best addressed by having a post merge test set.

things that fail frequently are worth the effort to develop unit tests for.

@oharboe
Copy link
Collaborator Author

oharboe commented Sep 9, 2025

I see that the bazel tests and build finish sooner than other CI work by at least 10 minutes, so there is no visible slowdown now.

@oharboe
Copy link
Collaborator Author

oharboe commented Sep 10, 2025

@maliberty Merge? As long as this is well under the other running times in the CI, can we merge this to cover more use-cases?

@maliberty
Copy link
Member

Its not just about performance. This isn't a flow test. Using large designs to test small bits of functionality is not a good methodology.

@oharboe
Copy link
Collaborator Author

oharboe commented Sep 10, 2025

Its not just about performance. This isn't a flow test. Using large designs to test small bits of functionality is not a good methodology.

Ideally yes, but do you have these tests today?

Integration tests are a result of accepting that fast unittests for everything and having a mix of fast unittests + integration is a false choice.

@oharboe oharboe closed this Sep 10, 2025
@maliberty
Copy link
Member

If we lack tests then they should be added as unit tests. We do have tests for write_pin_placement (in odb) and write_pin_placement (in ppl)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants