-
-
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
Open
llrs-roche
wants to merge
52
commits into
main
Choose a base branch
from
580_module_tests@main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 23 commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
62dee69
Add script to change testing depth of some tests
llrs-roche aff016a
Look up for only R or tests files (not vignettes, CI/CD or dependencies)
llrs-roche c142c72
Reorganize code
llrs-roche 7f5ed65
Merge branch 'main' into 580_module_tests@main
llrs-roche 7c1c12b
Add chekout action to be on the package repository
llrs-roche 58fa831
Merge branch '580_module_tests@main' of https://github.com/insightsen…
llrs-roche dcaedbf
Add emoji
llrs-roche efdc03b
Reset to the previous TESTING_DEPTH
llrs-roche 044ccf6
Uses and run aren't compatible
llrs-roche 161b1c6
Checkout to the right directory
llrs-roche 6e0c72a
Use path instead of working-directory
llrs-roche 9d31c30
Move the working directory where it should be
llrs-roche 25ce19d
Add some more info to help debug what is happening
llrs-roche b684dfc
Get the default branch of the repository to compare with.
llrs-roche 09dd369
Modify fetch-depth to have also the default branch
llrs-roche aa02b04
Check other folders
llrs-roche 31e58e7
Fetch all branches
llrs-roche decb120
Directly fetch the main branch from origin
llrs-roche 8c01893
Print remotes for debugging
llrs-roche 8f4afc7
Change quotes and to pull instead just fetch
llrs-roche bc4f2db
Add three dots and the branch name.
llrs-roche eb4b3b3
Fetch the branch specifically
llrs-roche 38b63de
Use only git-diff to compare between branches?
llrs-roche 76cfbf0
Undo changes and finish loop if any helper is modified
llrs-roche 5a5537d
Make it more general for other teal.* packages
llrs-roche 9e129a0
Address comments and provide informative log
llrs-roche 0517564
Add missing subprocess
llrs-roche f4a1bc8
Fix syntax errors
llrs-roche 65e160d
Further improvements
llrs-roche 642a189
Make it more general and improve logging
llrs-roche 3a58352
Restore only modifications on tests files
llrs-roche 48dcd52
Simplify a bit more
llrs-roche ab60d1c
Fix when to run this step!
llrs-roche cd1922d
Use logical/boolean instead of text
llrs-roche a27ebdf
Run target always not only on pull_requests
llrs-roche 6e0897f
Start from the beginning
llrs-roche 67c43fc
Set the working directory to the package
llrs-roche 38d02c2
Check if main commands are available
llrs-roche ef84cea
Change output and see what happens
llrs-roche 825ff79
Update description
llrs-roche 577fc84
Change bash
llrs-roche b843d4e
Fix issue with grep
llrs-roche dbe3631
Update script to be more informative
llrs-roche ccc51b3
Revert to head and fetch
llrs-roche f39761f
Fix path to git diff
llrs-roche 45b3b31
Pull branch before comparing it
llrs-roche 111855d
Pull directly from remote
llrs-roche bfd6310
setup merging strategy
llrs-roche ca95d1e
Stash before merging
llrs-roche abdbb86
Testing external action
llrs-roche ed42539
fix syntax
llrs-roche 123af45
Modify documentation and path
llrs-roche File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -294,7 +294,6 @@ jobs: | |
uses: actions/[email protected] | ||
if: github.event_name == 'pull_request' | ||
with: | ||
ref: ${{ steps.branch-name.outputs.head_ref_branch }} | ||
path: ${{ github.event.repository.name }} | ||
repository: ${{ github.event.pull_request.head.repo.full_name }} | ||
fetch-depth: 0 | ||
|
@@ -515,6 +514,52 @@ jobs: | |
path: "${{ inputs.additional-caches }}" | ||
key: additional-caches-${{ runner.os }} | ||
|
||
- name: Check only affected modules 🎯 | ||
if: startsWith(${{ github.event.repository.name }}, 'teal.modules.') && github.event_name == 'pull_request' | ||
run: | | ||
# Set working directory where the git repo is | ||
cd ${{ github.event.repository.name }}/${{ inputs.package-subdirectory }} | ||
|
||
# Debugging | ||
|
||
|
||
# Get the list of R files that have changed | ||
# https://github.com/cloudnativeto/envoy/blob/de67446fa8af973761bb4716f5143d73643baf8b/.github/workflows/get_build_targets.sh_bak#L9 | ||
files=$(git diff --name-only HEAD FETCH_HEAD -- tests/* R/*.R) | ||
|
||
# Base directories | ||
src_dir="R/" | ||
test_dir="tests/testthat/" | ||
|
||
# Loop through each modified file and determine which tests to run | ||
llrs-roche marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for file in $files; do | ||
|
||
# Extract the base name of the file, examples: | ||
# tests/testthat/test-shinytest2-foo.R -> foo | ||
# R/foo.R -> foo | ||
base_name=$(basename "$file" .R | sed s/test-shinytest2-//g) | ||
# Find matching test files (parenthesis to not match arguments) | ||
test_files=$(grep -l "$base_name(" "$test_dir"test-shinytest2-*.R) | ||
td=$TESTING_DEPTH | ||
|
||
# Modify in place so that only modified modules are tested. | ||
if [[ -n "$test_files" ]] | ||
then { | ||
sed -i 's/skip_if_too_deep(5)/skip_if_too_deep(3)/g' "$test_files" | ||
m7pr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
TESTING_DEPTH=3 | ||
} else { | ||
# Flag for helpers | ||
helper_modified="yes" | ||
} fi | ||
|
||
# R file without corresponding test file: reset the testing depth | ||
if [[ -n "$helper_modified" ]] | ||
then { | ||
TESTING_DEPTH=td | ||
} fi | ||
done | ||
shell: bash | ||
|
||
- name: Build R package 🏗 | ||
run: | | ||
if [ "${{ inputs.additional-env-vars }}" != "" ] | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@llrs-roche You created a step that gets triggered if a name of the package starts with
teal.modules.*
. We also have packages that have modules but do not havemodules
in the name, liketeal.osprey
orteal.goshawk
. Any way you can parametrize this step, so it can be turned on or off for specific repositories? By default it should be turned off, and we only turn it on on some repositories.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.
Thanks for raising this. I could check that this is only run when repository starts with
teal.*
.I will look into parametrizing it. However, the idea behind this PR is to reduce the number of tests or run everything. In the later case it would run as before adding this step.
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.
there are packages that do not start with
teal
that are in our ecosystem : P I think we need a flag/parameterThere 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.
I added an input parameter on the action
fast-tests
to enable this new step.