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

[Proposal] Add targeting to CI tests to streamline documentation changes #1524

Open
danielvegamyhre opened this issue Jan 8, 2025 · 4 comments
Labels
ci topic: improvement Use this tag if this PR is an improvement (doesn't fit into any of the other categories) triaged

Comments

@danielvegamyhre
Copy link
Contributor

danielvegamyhre commented Jan 8, 2025

I think it would be a nice developer workflow improvement/speedup if the CI tests had targeting rules, to selectively run tests based on the files included in the diff. The CI tests don't take too long right now, but as codebase and number of tests grow the CI will continue requiring more time + resources.

Examples:

  1. Most tests could not trigger on changes only touching self-contained prototypes in torchao/prototype that are not referenced/used elsewhere in the codebase.
  2. Most tests could not trigger on changes which only change documentation.

I believe jobs in Github actions can be configured to include/exclude certain paths (link).

I'm curious to hear others' thoughts on this, if anyone else thinks this would beneficial or not?

@danielvegamyhre danielvegamyhre added ci topic: improvement Use this tag if this PR is an improvement (doesn't fit into any of the other categories) labels Jan 8, 2025
@jcaip
Copy link
Contributor

jcaip commented Jan 8, 2025

cc @andrewor14 @jerryzh168

It looks from HUD the CI takes ~45 minutes to complete. So i don't think it's too bad but it would be nice. @andrewor14 do you know if we have any test time tracking for pytorch that we could see if we could use here? It we be good to know if there's a couple test that take a long time that we could prioritize adding first. Seems like a good BE project

@jcaip jcaip added the triaged label Jan 8, 2025
@jerryzh168
Copy link
Contributor

jerryzh168 commented Jan 8, 2025

Yeah I think it will be a nice feature, but I remember hearing that target determination is pretty hard to fully solve, is the proposal just to have the simple heuristics mentioned above to cover let's say 10% of cases, or is there a plan for a more sophisticated heuristic that can cover individual test cases as well? if it's at the file level, seems should be straightforward to do

@danielvegamyhre
Copy link
Contributor Author

danielvegamyhre commented Jan 8, 2025

Yeah I think it will be a nice feature, but I remember hearing that target determination is pretty hard to fully solve, is the proposal just to have the simple heuristics mentioned above to cover let's say 10% of cases, or is there a plan for a more sophisticated heuristic that can cover individual test cases as well? if it's at the file level, seems should be straightforward to do

Yeah I was thinking we could just start with some simple heuristics for easy cases like the ones I listed above. I'd imagine full target determination would require some combination of static analysis and other techniques to determine if code path executed in the test has any dependency changes. I'm actually not sure if this is possible just using Github actions, it might require more sophisticated CI focused software, I'd need to look into it.

@danielvegamyhre
Copy link
Contributor Author

I'm adding some simple heuristics to avoid doing the ~45min of CI tests for PRs which only update docs and make no code changes, to reduce friction involved in maintaining updated documentation.

However, doing full dependency analysis for testing seems tricky with github actions, best compatible tooling I can find is testmon (https://testmon.org/) which looks nice but lives in an individual guy’s github (https://github.com/tarpas/pytest-testmon), so I'm not confident about making it a critical part of CI.

IMO full dependency analysis / test targeting is a whole can of worms and currently doesn't seem impactful enough to justify the eng effort right now. I think reducing friction for maintaining up to date docs is a nice change and after that we can consider this issue complete.

@danielvegamyhre danielvegamyhre changed the title [Proposal] Add targeting to CI tests [Proposal] Add targeting to CI tests to streamline documentation changes Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci topic: improvement Use this tag if this PR is an improvement (doesn't fit into any of the other categories) triaged
Projects
None yet
Development

No branches or pull requests

3 participants