Adds --verify

[jspaezp]

Discussed here #153

The idea is to have a cli flag that works as --dry-run BUT returns a status 1 if any change would have been made on the files.

This PR is still a work in progress, right now its compatible with the behavior of the rest of the flags but I think we need to add a test case that verifies its behavior directly (2 files, one that gets changed, one that doesnt and make sure they exit with the correct error) an additional test I can think of is that no file should return status 1 if it has already been stripped (so .... step 1, verify it returns status 1, make sure no changes are made; step 2 run bstripout, step 3 verify that it returns status 0 on the modified file).

[kynan]

What's the reason for writing to a JSON formatted string here and below? Could you also just compare the dicts directly? Not necessarily objecting, just trying to understand why you've implemented it this way :)

[jspaezp]

I was afraid of instances where the dict is different but the serialization is the same; for instance if for some reason the strip zepellin converts a list to a tuple. Both are equivalent in json but not as python dicts. (It might also prevent some edge cases where copying vs deep cloning might be an issue)

Example

import json

d1 = {"a": 1, "b": [1,]}
d2 = {"a": 1, "b": (1,)}

print(json.dumps(d1) == json.dumps(d2))
# True
print(d2 == d1)
# False
print(json.dumps(d2))
# {"a": 1, "b": [1]}

(I am not 100% sure if my concerns are totally grounded, but felt better).

[kynan]

You are indeed correct that both strip_output and strip_zeppelin_output mutate the existing dict, so we'd probably need to create a deep copy for comparison purposes. I'd prefer that over the JSON serialization.

[jspaezp]

Done!

[kynan]

You are indeed correct that both strip_output and strip_zeppelin_output mutate the existing dict, so we'd probably need to create a deep copy for comparison purposes. I'd prefer that over the JSON serialization.

[sh0rtcircuit]

@jspaezp Are there any updates on this? It would be super-helpful if this PR could be completed =)

[jspaezp]

@sh0rtcircuit Thanks for the nudge!
I added the suggestions in a51d43f

Also took care of updating to the main branch.
@kynan LMK If there is anything else you would like done!

[kynan]

Nearly there :)

[kynan]

2 minor nits, then we should we good once you've addressed the failing tests :)

[kynan]

Suggested change

	if input_ != expected:
	assert pc.returncode == 1
	else:
	assert pc.returncode == 0
	assert pc.returncode == 1 if input_ != expected else 0

[jspaezp]

Note: This the tests really hard to figure out and I partially reverted the change bc it is equivalent to ...

>>> (pc.returncode == 1) if input_ != expected else (0)

instead of

>>> pc.returncode == (1 if input_ != expected else 0)

[kynan]

Suggested change

	if verify:
	assert pc.returncode == 1
	else:
	assert pc.returncode == 0
	assert pc.returncode == 1 if verify else 0

[kynan]

@jspaezp Unfortunately tests are still failing :(

[kynan]

Thanks, that's looking good now and tests are passing. The only thing missing now is documentation. Are you happy to document this in the README? Sorry this has been dragging on for so long :(

[jspaezp]

Updated the readme on f5b02d2

[kynan]

Thanks for your contribution! This is ready to merge now 👍🏽

[sh0rtcircuit]

Guys you are amazing thanks a lot !! 👏😃