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

Extend simulation infrastructure with Now() #509

Merged
merged 9 commits into from
Dec 20, 2023

Conversation

lekcyjna123
Copy link
Contributor

@lekcyjna123 lekcyjna123 commented Nov 11, 2023

In some places it is useful to have direct access to the current cycle number. Till now we could manually count cycles, but this required using call_try everywhere. This commit ports simulation framework extension from #395. There is added new class Now() which can be yielded during testing. In return a current cycle will be returned.

This feature is needed to extend and port CondVar from #395

coreblocks/test/common.py

Lines 429 to 433 in ddb21b5

class CondVar:
"""
Simple CondVar. It has some limitations e.g. it can not notify other process
without waiting a cycle.
"""

Current CondVar notification requires to wait a cycle on notify. With usage of Now() it should be possible to omit this waiting.

test/common/infrastructure.py Outdated Show resolved Hide resolved
test/common/infrastructure.py Outdated Show resolved Hide resolved
Copy link
Member

@Kristopher38 Kristopher38 left a comment

Choose a reason for hiding this comment

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

LGTM, left one non-critical question

Comment on lines +8 to +9
if TYPE_CHECKING:
from .infrastructure import CoreblocksCommand
Copy link
Member

Choose a reason for hiding this comment

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

Could you elaborate why this needs to be conditioned on typechecking?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because in infrastructure.py file I import symbols from functions.py. So without this if there is a circular dependency.

@lekcyjna123 lekcyjna123 force-pushed the lekcyjna/add-common-now branch from 432fa00 to 04a6015 Compare November 19, 2023 17:40
This was referenced Nov 19, 2023
@lekcyjna123 lekcyjna123 requested a review from tilk December 15, 2023 12:10
Copy link
Member

@tilk tilk left a comment

Choose a reason for hiding this comment

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

Let's finally merge some testing extensions.

@tilk tilk merged commit 2d5c9a6 into kuznia-rdzeni:master Dec 20, 2023
8 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 20, 2023
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.

3 participants