Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Resolve issue 39 #50

Merged
merged 4 commits into from
Oct 4, 2021
Merged

Resolve issue 39 #50

merged 4 commits into from
Oct 4, 2021

Conversation

yy665
Copy link
Collaborator

@yy665 yy665 commented Sep 29, 2021

@dz333

Changed e.to.mergeStmts(nstmts, false) to allow recv stmt gets prepended.

@dz333
Copy link
Collaborator

dz333 commented Sep 30, 2021

Great! Can you add a test to the src/test/tests/branchesCheck folder with the example from the issue (or something similar)?
See the other tests in that directory for how to specify solution files (that go in src/test/tests/branchesCheck/solutions) and memory files to specify the memory initial state for simulation (src/test/tests/branchesCheck/memInputs).

Once you add the test file I can also help make sure it's running properly.

@yy665
Copy link
Collaborator Author

yy665 commented Sep 30, 2021

Great! Can you add a test to the src/test/tests/branchesCheck folder with the example from the issue (or something similar)? See the other tests in that directory for how to specify solution files (that go in src/test/tests/branchesCheck/solutions) and memory files to specify the memory initial state for simulation (src/test/tests/branchesCheck/memInputs).

Once you add the test file I can also help make sure it's running properly.

Thanks for the feedback.

I have added some test files. Not sure if these tests are good. I tried to run the tests locally but there are some issues with my local bsc simulator that I might have to reinstall.

Copy link
Collaborator

@dz333 dz333 left a comment

Choose a reason for hiding this comment

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

Looks good! The only issue is that your solution file for the simulation has the wrong number of spaces (that's why the test is failing). The simulator right aligns numbers based on the number of bits in their type.

@dz333 dz333 merged commit fb0f366 into apl-cornell:main Oct 4, 2021
@dz333
Copy link
Collaborator

dz333 commented Oct 4, 2021

I've merged it into main with a fix for the solution file

@yy665
Copy link
Collaborator Author

yy665 commented Oct 4, 2021

Thanks! @dz333

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants