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

Fix bug in test.sh counterexample error matcher 🐞 #288

Merged
merged 2 commits into from
Oct 2, 2024
Merged

Conversation

vcschapp
Copy link
Collaborator

@vcschapp vcschapp commented Oct 2, 2024

Category

What kind of change is this?
Please select one of the following four options.
Consult Pull request merging criteria for a description of each category.

  1. Cosmetic change.
  2. Documentation change by member.
  3. Documentation change by Overture tech writer.
  4. Material change.

Description

Brief description of the business purpose and effect of the pull request.

This commit fixes a bug in the test.sh script that made it give false positive test passes on all counterexamples. It then fixes all the counterexamples to ensure they pass with the fixed script. 😰

The genesis of the test.sh bug was the output changes in the jv tool. We've been working to fix our testing setup in response to the jv changes since around June 9, 2024 with commit 8071ea8b, but there's been a bit of whack-a-mole as fixes for problems beget new problems. Hopefully this is the last one in the series!

Reference

List of relevant links to GitHub issues, PRs, and other documentation.

  1. Fix jv #211.
  2. Fix issues with test.sh #214

Testing

Brief description of the testing done for this change showing why you are confident it works as expected and does not introduce regressions. Provide sample output data where appropriate.

  1. Ran the counterexamples after changing test.sh and found they were almost all broken.
  2. Fixed them.
  3. Re-ran the counterexamples.

Checklist

Checklist of tasks commonly-associated with schema pull requests. Please review the relevant checklists and ensure you do all the tasks that are required for the change you made.

  1. Add relevant examples.
  2. Add relevant counterexamples.
  3. Update any counterexamples that became obsolete. For example, if a counterexample uses property A but is not intended to test property A's validity, and you made a schema change that invalidates property A in that counterexample, fix the counterexample to align it with your schema change.
  4. Update in-schema documentation using plain English written in complete sentences, if an update is required.
  5. Update Docusaurus documentation, if an update is required.
  6. Review change with Overture technical writer to ensure any advanced documentation needs will be taken care of, unless the change is trivial and would not affect the documentation.

Documentation Website

Update the hyperlink below to put the pull request number in.

Docs preview for this PR.

As a result of this bug, no counterexample checks have been run April
24, 2024 (commit `16ed4351`).
This commit ensures that all counterexample tests pass after applying
the `test.sh` fixes from the previous two commits.
@vcschapp
Copy link
Collaborator Author

vcschapp commented Oct 2, 2024

Talked in Schema TF, not a material change, needs own bucket same approval criteria as Cosmetic but different content. Or expand cosmetic definition?

Also, pls stick "Schema" between "Material and Change" in the criteria.

@vcschapp vcschapp merged commit d1b4b5a into dev Oct 2, 2024
2 checks passed
@vcschapp vcschapp deleted the fix_tests_2 branch October 2, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants