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

ci: Enalbe tests with no platform specified #167

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

TimePrinciple
Copy link
Contributor

@TimePrinciple TimePrinciple commented Oct 3, 2024

Summary of the PR

After .platform mechanism was introduced in #159, it silently filters
out tests with no platform specified. Enable them to execute by adding
platform is not None predicate preceeds allowlist check.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

@TimePrinciple
Copy link
Contributor Author

TimePrinciple commented Oct 3, 2024

I'm deeply sorry about breaking our CI tests 😭

roypat
roypat previously approved these changes Oct 3, 2024
Copy link
Collaborator

@roypat roypat left a comment

Choose a reason for hiding this comment

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

Ah, well, its just the commit-format test. No big deal, thanks for noticing and fixing!

@TimePrinciple
Copy link
Contributor Author

Ah, well, its just the commit-format test. No big deal, thanks for noticing and fixing!

And style test as well :(

@roypat
Copy link
Collaborator

roypat commented Oct 3, 2024

Ah, well, its just the commit-format test. No big deal, thanks for noticing and fixing!

And style test as well :(

Ah true. Still, out of all the tests it could've hit, these were definitely the least important ones, so don't beat yourself up over it! It's just as much our fault for not noticing it during review

After `.platform` mechanism was introduced in rust-vmm#159, it silently filters
out tests with no `platform` specified. Enable them to execute by adding
`platform is not None` predicate preceeds allowlist check.

Signed-off-by: Ruoqing He <[email protected]>
@roypat roypat merged commit 92034f0 into rust-vmm:main Oct 7, 2024
3 checks passed
@TimePrinciple TimePrinciple deleted the enable-no-platform-tests branch October 7, 2024 07:05
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.

3 participants