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 tests in master branch #82

Merged
merged 8 commits into from
Sep 5, 2024
Merged

Fix tests in master branch #82

merged 8 commits into from
Sep 5, 2024

Conversation

p12tic
Copy link
Contributor

@p12tic p12tic commented Sep 5, 2024

Currently CI tests are broken on master branch due to various reasons. #78 attempted to address at least some of the problems.

The current PR builds on top of #78 and fixes the remaining issues.

cc @cmarqu

@cmarqu cmarqu requested a review from ktbarrett September 5, 2024 06:51
cocotb has deprecated Join with the message "Using task directly is
prefered to Join(task) in all situations where the latter could be
used.". Awaiting on task is possible in all places where cocotb-bus
currently uses Join.

Unfortunately the newer approach is not supported on older versions of
cocotb. Accordingly two code paths have been introduced in the
problematic place. It is a test for cocotb-bus itself, therefore there's
no performance impact to check versions there.
There is no constraint on tb.stop() action with regards to all other
coroutines waiting for clock rising edge. This may lead to mismatching
transactions and expectations being sent to Scoreboard and the following
error:

AssertionError: Received a transaction but wasn't expecting anything

Waiting on ReadOnly trigger ensures that all other coroutines have
performed their actions before the testbetch is stopped.
@ktbarrett ktbarrett merged commit cf1e834 into cocotb:master Sep 5, 2024
5 checks passed
@p12tic p12tic deleted the fix-tests branch September 5, 2024 17:11
@p12tic p12tic mentioned this pull request Sep 5, 2024
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.

4 participants