-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
Run shinytests2 only if the modules are affected #273
Conversation
When a module file or a test file is modified this PR lowers the testing threshold environmental variable when the threshold of the test file is lowered too. Any change on the R files that are not found on I have created a local branch on tmc to test this, and also tested it on teal.code.
|
Thanks @walkowif and @cicdguy for your help, I'd appreciate further help to finish this PR. I don't think I can't take this further and it would be a really great feature to have before tmg and tmc releases. The key points of the new step are described on a comment: we want to make checks faster by only testing what have been modified. I think the bash script logic is correct, but I cannot test if it works well on the testing branch for tmc. There are differences between the local repository and the repository created by the github action. These differences make it difficult for me to resolve them. Could you modify this branch so that the testing branch reaches the end of the small bash script? |
Maybe you could try using this GH Action instead to get the files modified in a given PR? The example in the readme shows how to loop over the changed files so that you could process them according to the logic you already have in your script. |
Thanks for the suggestion, using that external action is in my opinion risky: It is updated quite often and it is not clear to me what does on each action (the behavior seems a bit different between main and a PR). In addition I don't know how well it will work with the changes on the repository done by the action itself. I am testing it but is up to you. |
@llrs-roche - yes there is a risk with using external actions. The same applies to using anything in the open source community. We already this and many other external actions in our workflows so it's okay for us to assume the risk. Also, actions are versioned so make sure you pin the version when you use it. |
@cicdguy The problem is not with using an open source action but on how stable it is. If it needs many updates are you okay with using it with security problems? Or updating every X months? |
Yes, and that's why I'm asking you to pin the version. If the version is pinned, you're going to end up using that specific version, right? Then the risk of encountering an API change will be minimal.
Yes, we do this anyway. |
Many thanks all for your feedback. This branch is ready for another round of reviews.
I'd appreciate your feedback @m7pr and @cicdguy. Some questions I would make or comment as a reviewer.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine. Good to go.
Pull Request
Fixes: https://github.com/insightsengineering/coredev-tasks/issues/580
This should make testing on PR to main a lot faster by just running test of the modules affected.
TODO:
@dev
?