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

Discuss: Too many positive tests? #4

Open
plocket opened this issue Oct 8, 2023 · 3 comments
Open

Discuss: Too many positive tests? #4

plocket opened this issue Oct 8, 2023 · 3 comments
Labels
question Further information is requested

Comments

@plocket
Copy link
Owner

plocket commented Oct 8, 2023

Description of the JSON schema.

I went into the da repo and got all the examples. I then put them all in one file and parsed out and removed blocks that had duplicate root properties. It ended up leaving out things like multiselect and such, so it obviously removed a bit too much.

Still, it produced 218 blocks. Is that too many? It feels like too many. On the other hand, I'm not sure how to pick which blocks to exclude, especially without going through them all by hand.

Here they all are: https://gist.github.com/plocket/cc377ce980b5a088a7f6ee4cf411b48f

@BryceStevenWilley if you're interested

Supporting information.

I won't support this!

Are you making a PR for this?

Yes, I will create a PR.

@plocket plocket added the question Further information is requested label Oct 8, 2023
@BryceStevenWilley
Copy link
Collaborator

I don't think this can have too many tests! TBH. I've been locally testing on those and more (a lot of the repos from Suffolk too). What did you mean by duplicate root properties though?

If you're okay with it, I think we should just be able to point the tests at the docassemble repo itself to test; if we make a GH action for tests, we can call actions/checkout on the docassemble repo, and we could document the same procedure for manually testing.

@plocket
Copy link
Owner Author

plocket commented Oct 10, 2023

What did you mean by duplicate root properties though?

I mean there are a bunch of blocks that just have, e.g.:

mandatory: True
code: |
  some_code

For just testing the JSON schema, we don't need all of those individual tests since they would test the same keys, so I weeded out the extras. I should put that script somewhere. Maybe the tests are fast enough that it won't matter. Note that those examples might come in handy if we end up trying to test out the code written in code:, since we'll probably want to check for [i] and such.

As I've been building up the schema, though, I've been finding that the tests in da examples probably won't be thorough enough. They don't go through permutations and they don't create negative tests (tests that should fail). I think we'll end up writing a lot of tests ourselves anyway as we build up each key's specs, so maybe that's just part of the process.

I do like the idea of using actions to get all the examples as part of the CI testing. Note for future us that we need to weed out bad files, like binary files (of which there is at least one). Probably there's an easy way.

@BryceStevenWilley
Copy link
Collaborator

Maybe the tests are fast enough that it won't matter

They should be; this should be a tool that can run each time a file changes, not quite every key stroke, but every 5-10 seconds for sure. If it starts taking too long we can consider chaning things, but I think the default should be everything.

I agree about adding our own tests though. Definitely something to consider.

I have an ad hoc list of yaml files in the DA repo that aren't interview files here: https://github.com/BryceStevenWilley/DAYamlStructure/blob/main/da_yml_structure.py#L435, in case that helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants