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

Either drop file/size from add/autoadd OR add md5 and sha to it. #262

Closed
aelsabbahy opened this issue Jul 10, 2017 · 8 comments · Fixed by #814
Closed

Either drop file/size from add/autoadd OR add md5 and sha to it. #262

aelsabbahy opened this issue Jul 10, 2017 · 8 comments · Fixed by #814

Comments

@aelsabbahy
Copy link
Member

Need to make a decision on which one of these attributes are included in add and which are not:

  • Size
  • md5
  • sha256
@pysysops
Copy link
Contributor

pysysops commented Aug 6, 2017

SHA-256 may be the most sensible and current attribute to default to? Especially with the recent (last year or so) posts about SHA-1 / MD5 collisions.

@aelsabbahy
Copy link
Member Author

If one of them were to stay sha256 makes the most sense.

I'm just not sure if sha256 by default is a useful addition or an annoyance (generated test too rigid).

I can go either way on this one. Drop them all, add them all, or only keep sha256.

@elliotweiser
Copy link
Contributor

When deciding on defaults, it's often best to go the "simpler" route.

IMO, it's best to use none of those checks and to let the user decide what he/she wants to add.

Perhaps this opens the door on supporting additional flags to goss add for the file test, e.g. --checksum. Thoughts?

@aelsabbahy
Copy link
Member Author

So the opposite exists today in Goss with --exclude-attr. There could be an --include-xtra-attr global flag that does this.

That said, to your point about "simpler." It really comes down to what would the majority of users expect from a goss add file. My gut tells me @pysysops is right with just sha256, but no size.

@elliotweiser
Copy link
Contributor

I think it comes down to whether or not a file is expected to change very often. While I enjoy a special love for the sha256 test, I cannot claim one way or the other that static files are the target of the majority use case.

@aelsabbahy
Copy link
Member Author

Yeah, that's what bother me about size. It's such a flakey test, but with even less value than sha256.

Maybe less is more. Keep all the "extra" tests as manual.

@elliotweiser
Copy link
Contributor

FWIW, I wouldn't be opposed to switching out size with sha256 to see what kind of feedback it generates.

@aelsabbahy
Copy link
Member Author

Deciding to drop size

@aelsabbahy aelsabbahy added the v4 label Jul 16, 2020
@aelsabbahy aelsabbahy mentioned this issue Nov 13, 2020
Closed
5 tasks
aelsabbahy added a commit that referenced this issue Dec 30, 2020
* Add transforms

* wip add transforms work

* Add some examples to make sense of this

* Handle floats/ints better

* Convert validatecontails to gomega

* wip

* Add gjson and contain-substring

* wip

* Subclass all matchers adding String()

* I don't evne know what this is.. should have checked it in back when I wrote it

* Migrate GomegaMatcher -> GossMatcher

* lots of changes + drop json-iterator

* Move output related logic to outputs

* Add include_raw flag

* Add bench output format

* Update have_patterns to check input. Add stubs to make Not matcher act as omegaMatcher

* SetEscapeHTML(false) for semver constraint

* Ensure have-Key_with_value works

* remove have-key-with-value

* Fix error compact output. Add oMegaMatcher stubs. Ensure matcher vs m consistent

* Cleanup matchers

* Cleanup TestResult struct and remove need for dyno

* Update all deps

* Update tranformer tests and add summary line to bench output format

* Update readme

* Conditionally print missing and empty values. Sanitize found and expected values for consistency

* Check value is valid before checking IsNil()

* Initial docs change

* WIP fix for pretty print

* Swap order on video vs blog for dgoss docs

* Updated all tests except for semver

* Fix all unit tests

* Fix detection of when matcher is set

* Update documentation

* Update docs/manual.md

Co-authored-by: Peter Mounce <pete@neverrunwithscissors.com>

* Update docs/manual.md

Co-authored-by: Peter Mounce <pete@neverrunwithscissors.com>

* gjson: Validate json before doing gets

* Make errors more clear

* Remove unused field

* Bump go version

* Update doc

* Fix regression with gomega errors

* Fix #262 no need to add file size by default

* Fix output formats

* Use proper quoting for windows test

* Add error checking for gjson path not found

Co-authored-by: Peter Mounce <pete@neverrunwithscissors.com>
@aelsabbahy aelsabbahy mentioned this issue Jun 25, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants