Skip to content

Commit

Permalink
turn off coverage testing
Browse files Browse the repository at this point in the history
this reverts previous breakage of coverage functionality.
coverage functionality is now turned off for all circleci jobs
  • Loading branch information
leej3 committed Jun 12, 2021
1 parent 9a8c0aa commit dfe88b8
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 9 deletions.
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,6 @@ workflows:
test_data_cache_version: ["5"]
filters:
tags:
only: /AFNI_\d\d.\d.\d\d/
ignore: /.*/
branches:
only: /.*/
ignore: /.*/
5 changes: 1 addition & 4 deletions tests/afni_test_utils/run_tests_func.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,7 @@ def run_tests(tests_dir, **args_dict):
# append gcovr to assemble coverage report for C code
cmd += f"; gcovr -s --xml -o {tests_dir}/gcovr_output.xml -r {args_dict['build_dir']}/src"
# append command for compiling and uploading codecov report

# apparently there is a security issue here, must investigate
# cmd += "; bash -c 'bash <(curl -s https://codecov.io/bash)'"
# sys.exit(1)
cmd += "; bash -c 'bash <(curl -s https://codecov.io/bash)'"

print(f"Executing: {cmd}")
res = subprocess.run(cmd, shell=True, env=os.environ.copy())
Expand Down
4 changes: 1 addition & 3 deletions tests/scripts/test_testing_script_functionality.py
Original file line number Diff line number Diff line change
Expand Up @@ -718,9 +718,7 @@ def test_run_tests_container_subparsers_works(monkeypatch, argslist, mocked_scri
"ARGS='{DEFAULT_ARGS} {PYTEST_COV_FLAGS}' "
"ninja pytest;"
" gcovr -s --xml -o {TESTS_DIR}/gcovr_output.xml -r {params['args_in']['build_dir']}/src;"
" echo ======= REFUSING TO GO TO codecov.io ======== "
# there may be a security issue with getting the script this way
# " bash -c 'bash <(curl -s https://codecov.io/bash)'"
" bash -c 'bash <(curl -s https://codecov.io/bash)'"
),
},
],
Expand Down

3 comments on commit dfe88b8

@mrneont
Copy link
Contributor

Choose a reason for hiding this comment

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

@leej3

Why does this need to be turned on? We (including @afni-rickr ) were alerted about severe codecov security issues, and we turned it off because of that. Isn't codecov separate from CircleCI, and so can't we leave it off for testing for now? If not, why not? (And if not, can we change things so it can be?)

@mrneont
Copy link
Contributor

Choose a reason for hiding this comment

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

Or am I misreading what this commit does? The commit string is "turn off codecov..." but this commit uncomments a commenting out that we had made to definitely turn it off. Sorry if I am misinterpreting, but due to the security risks of CodeCov, we have to be careful.

@leej3
Copy link
Collaborator Author

@leej3 leej3 commented on dfe88b8 Jun 13, 2021

Choose a reason for hiding this comment

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

Yes it turns off the code coverage testing but the functionality is left intact so that tests of that functionality do not fail. It also means that people can still run coverage testing locally if they wish.

Please sign in to comment.