Skip to content

dencoder tests fix type backwards incompatible checks #60945

Merged
SrinivasaBharath merged 2 commits intoceph:mainfrom
NitzanMordhai:wip-nitzan-crushwrapper-corpus-squid
Feb 3, 2025
Merged

dencoder tests fix type backwards incompatible checks #60945
SrinivasaBharath merged 2 commits intoceph:mainfrom
NitzanMordhai:wip-nitzan-crushwrapper-corpus-squid

Conversation

@NitzanMordhai
Copy link
Contributor

@NitzanMordhai NitzanMordhai commented Dec 4, 2024

This PR addresses issues in the dencoder tests related to backward compatibility checks.

It improves the handling of type skipping and testing logic by ensuring compatibility is properly checked across versions. Types introduced in newer versions are now correctly skipped for older decoder versions, following cascading backward compatibility rules.

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

@github-actions github-actions bot added the tests label Dec 4, 2024
@NitzanMordhai NitzanMordhai force-pushed the wip-nitzan-crushwrapper-corpus-squid branch from 4c31663 to 15a4758 Compare December 4, 2024 15:55
Copy link
Member

@ljflores ljflores left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a related make check failure:

workunits/dencoder/test_readable.py:17: error: Need type annotation for "fast_shouldnt_skip" (hint: "fast_shouldnt_skip: List[<type>] = ...")  [var-annotated]
Found 1 error in 1 file (checked 287 source files)
mypy: exit 1 (18.56 seconds) /home/jenkins-buil

should_skip_object function needed to handle cascading backward
compatibility checks.
I also added improved handling of backward compatibility filtering
in should_skip_object so we can better find backward comp. and skip
them.

Fixes: https://tracker.ceph.com/issues/69009
Signed-off-by: Nitzan Mordechai <[email protected]>
ceph-object-corpus submodule

Signed-off-by: Nitzan Mordechai <[email protected]>
@NitzanMordhai NitzanMordhai force-pushed the wip-nitzan-crushwrapper-corpus-squid branch from 15a4758 to 882eaa7 Compare December 10, 2024 06:23
@NitzanMordhai
Copy link
Contributor Author

There is a related make check failure:

ohh miss that one, fixed

@NitzanMordhai
Copy link
Contributor Author

jenkins test api

Copy link
Member

@ljflores ljflores left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NitzanMordhai the "ceph-object-corpus" submodule was updated- I don't think that was intentional?

@NitzanMordhai
Copy link
Contributor Author

@NitzanMordhai the "ceph-object-corpus" submodule was updated- I don't think that was intentional?

yes, we have updated ceph-object-corpus branch, we need to update ceph with the submodule

@amathuria
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants