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

Prevent new uses of old ztest API #46824

Closed
yperess opened this issue Jun 23, 2022 · 8 comments
Closed

Prevent new uses of old ztest API #46824

yperess opened this issue Jun 23, 2022 · 8 comments
Labels
Process Tracked by the process WG

Comments

@yperess
Copy link
Collaborator

yperess commented Jun 23, 2022

Discussed in the TSC meeting, we need a way to prevent new uses of the ztest API.

I believe that the best way to do this will be to add a new check for PRs similar to a language filter that checks for inappropriate language. To do this we'd need:

  1. Create a new repo something like zephyrproject-rtos/word-blocklist
  2. Provide an implementation of the GitHub check that scans for new test suites:
    a. ztest_register_test_suite
    b. ztest_test_suite

I don't believe that we should block adding more tests to existing suites. That will automatically happen as people migrate to the V2 API.

Once we're done, zephyrproject-rtos/word-blocklist can remain and we can use this for other migrations. It's also possible that we can find an existing GitHub CI for profanity filter and simply fork it. I don't think it'll be a bad idea anyway to have a profanity filter.

@yperess yperess added the Process Tracked by the process WG label Jun 23, 2022
@yperess
Copy link
Collaborator Author

yperess commented Jun 23, 2022

@nashif

@yperess
Copy link
Collaborator Author

yperess commented Jun 23, 2022

@aaronemassey

@teburd
Copy link
Collaborator

teburd commented Jun 28, 2022

The docs likely need to be updated to use only newer methods as a first step for this. The docs at https://docs.zephyrproject.org/latest/develop/test/ztest.html still shows samples and such with what I guess should no longer be allowed.

@mbolivar-nordic
Copy link
Contributor

Process WG:

  • @mbolivar-nordic: how does a word blocklist allow you to say "the $BLOCKED_WORD API is now deprecated"?
  • @MaureenHelm : not sure why we don't use the regular deprecation process -- is there something special about this?

@yperess can you attend next week to discuss further if there's still something to handle here?

@yperess
Copy link
Collaborator Author

yperess commented Sep 21, 2022

Process WG:

  • @mbolivar-nordic: how does a word blocklist allow you to say "the $BLOCKED_WORD API is now deprecated"?
  • @MaureenHelm : not sure why we don't use the regular deprecation process -- is there something special about this?

This is for long migrations that take time. If we deprecate the old API our CI will break. We just want to prevent new uses.

@yperess can you attend next week to discuss further if there's still something to handle here?

Sure

@mbolivar-nordic
Copy link
Contributor

Process WG: defer; @yperess couldn't make it.

@nashif next steps should be "complete, deprecate, remove;" I'd like to year @yperess 's points about this, though.

@yperess
Copy link
Collaborator Author

yperess commented Sep 28, 2022

Sorry, I was stuck in another meeting. The idea of this workflow was to provide the following:

  1. We have a new API that Zephyr itself is in the process of migrating to. We cannot mark the old API as deprecated or it'll break our own CI.
  2. We use the blocklist CI tool to prevent new uses of the old API.
  3. We complete the migration of the old API to the new one (because new uses of the old API are prevented by the blocklist we don't have to keep chasing)
  4. We mark the old API deprecated

This issue was specific to the ztest API migration, and that's already got a lot of traction. I'm happy to just close this issue and ignore it, but I do think that this will come up again so it might be nice to have the mechanism in place.

@mbolivar-nordic
Copy link
Contributor

OK, I'lll close this for now and we can see if we need to reopen in the future. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Process Tracked by the process WG
Projects
Status: Done
Development

No branches or pull requests

3 participants