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

ioreg/test: Add test for #140 #148

Merged
merged 1 commit into from Aug 28, 2014

Conversation

Projects
None yet
3 participants
@bgamari
Contributor

bgamari commented Aug 28, 2014

write-only registers shouldn't be read from.

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 28, 2014

Maybe you can include #149 and close it?

@bgamari bgamari force-pushed the bgamari:ioreg-test branch from 9e3daad to 7b81fc4 Aug 28, 2014

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 28, 2014

@bharrisau done.

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 28, 2014

Arg, unfortunately the lint doesn't like hyphens either. Perhaps describe! should be doing more cleaning of identifiers?

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 28, 2014

This test is only making sure that write-only registers can compile. It doesn't test for reads - you need a spy for that - need to swap out the VolatileCell for a VolatileCellSpy.

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 28, 2014

Or otherwise modify VolatileCell to enable spying.

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 28, 2014

@bharrisau I'm not sure I follow; no functionality beyond what we already have should be necessary here. The current patch verifies that only the bits modified in the most recent write operation are set. This indirectly ensures that a given operation does not read from the bits which it does not touch.

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 28, 2014

Ahh - I get it now. The second one would fail if it was a read-modify-write.

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 28, 2014

Ok - r+ when you are ready.

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 28, 2014

@bharrisau I think this is ready unless you disagree with my assessment of describe!'s name sanitation.

@bgamari bgamari added ready and removed in progress labels Aug 28, 2014

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 28, 2014

Can you just remove the hyphen for now and call it writeonly, otherwise just allow non_snake_case in the test module.

I think shiny should include non_snake_case.

ioreg/test: Add test for #140
Write-only registers shouldn't be read from.

Also remove parentheses from test names as this upsets the snake-case
lint.

@bgamari bgamari force-pushed the bgamari:ioreg-test branch from 7b81fc4 to 4e83e42 Aug 28, 2014

bgamari added a commit that referenced this pull request Aug 28, 2014

Merge pull request #148 from bgamari/ioreg-test
ioreg/test: Add test for #140

@bgamari bgamari merged commit b9f6ffd into hackndev:master Aug 28, 2014

1 check was pending

continuous-integration/travis-ci The Travis CI build is in progress
Details

@bgamari bgamari deleted the bgamari:ioreg-test branch Aug 28, 2014

@bgamari bgamari restored the bgamari:ioreg-test branch Aug 28, 2014

@bgamari bgamari deleted the bgamari:ioreg-test branch Aug 28, 2014

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