mina icon indicating copy to clipboard operation
mina copied to clipboard

Fix ppx_version tests

Open georgeee opened this issue 9 months ago • 1 comments

Problem: ppx_version tests do not pass.

Solution: fix bounded_types usage and comment out a test that can't be trivially fixed.

Ideally, this commit is followed by another one that uncomments the bad_version_syntax_missing_versioned test execution.

Explain how you tested your changes:

  • Ran the ppx_version tests via Makefile

Checklist:

  • [x] Dependency versions are unchanged
    • Notify Velocity team if dependencies must change in CI
  • [x] Modified the current draft of release notes with details on what is completed or incomplete within this project
  • [x] Document code purpose, how to use it
    • Mention expected invariants, implicit constraints
  • [x] Tests were added for the new behavior
    • Document test purpose, significance of failures
    • Test names should reflect their purpose
  • [ ] All tests pass (CI will check this if you didn't)
  • [x] Serialized types are in stable-versioned modules
  • [x] Does this close issues? None

georgeee avatar May 08 '24 08:05 georgeee

@nholland94 could you explain your motivation for the change https://github.com/MinaProtocol/mina/pull/15195/files#diff-4639a9ab80fa8921be722da5501c054606e7c4cea41eab2134f9669492f052ee (part of #15195)?

It is the reason for test bad_version_syntax_missing_versioned failing. AFAIU, we do not check for %%versioned wrapping existence after your commit (and this is why the test is failing).

If this is intentional, docs have to be updated. And also function is_versioned_type_lident (and maybe some others too) are to be removed as unused code.

It seems to me though that the change wasn't intentional. Is there a problem if I just revert your whole change to src/lib/ppx_version/lint_version_syntax.ml?

georgeee avatar May 08 '24 08:05 georgeee

Created a follow-up issue #15809.

Making the PR a non-draft.

georgeee avatar Jul 10 '24 11:07 georgeee

!ci-build-me

georgeee avatar Jul 12 '24 09:07 georgeee