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

Fixing full test trigger to properly handle file changes in the root dir #157

Merged
merged 1 commit into from
Mar 16, 2020

Conversation

dry923
Copy link
Member

@dry923 dry923 commented Mar 11, 2020

Previously the full test trigger was not being properly executed if a file was changed in the root of the snafu directory. This fixes that.

@dry923 dry923 added the ok to test Kick off our CI framework label Mar 11, 2020
@rht-perf-ci
Copy link

Results for SNAFU CI Test

Test Result Runtime
fio_wrapper PASS 00:16:05
fs_drift_wrapper PASS 00:09:58
hammerdb PASS 00:11:16
iperf PASS 00:06:46
pgbench-wrapper PASS 00:07:15
smallfile_wrapper PASS 00:10:51
sysbench PASS 00:04:54
uperf-wrapper PASS 00:57:17
ycsb-wrapper PASS 00:17:37

Copy link
Member

@rsevilla87 rsevilla87 left a comment

Choose a reason for hiding this comment

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

@dry923 I think there's a easier solution for this issue that consists in just double quoting the ${diff_list} variable.
Example here:

$ diff_list=`git diff origin/master --name-only`
$ echo ${diff_list} | grep -cv /
0
echo "${diff_list}" | grep -cv /
1

# Double quoting, basically prints the original value of the variable

$ echo ${diff_list}
ci/run_ci.sh run_snafu.py smallfile_wrapper/README.md smallfile_wrapper/smallfile_wrapper.py smallfile_wrapper/sync-diagram.png smallfile_wrapper/trigger_smallfile.py
$ echo "${diff_list}" 
ci/run_ci.sh
run_snafu.py
smallfile_wrapper/README.md
smallfile_wrapper/smallfile_wrapper.py
smallfile_wrapper/sync-diagram.png
smallfile_wrapper/trigger_smallfile.py

What do you think?

@dry923
Copy link
Member Author

dry923 commented Mar 12, 2020

@dry923 I think there's a easier solution for this issue that consists in just double quoting the ${diff_list} variable.
Example here:

$ diff_list=`git diff origin/master --name-only`
$ echo ${diff_list} | grep -cv /
0
echo "${diff_list}" | grep -cv /
1

# Double quoting, basically prints the original value of the variable

$ echo ${diff_list}
ci/run_ci.sh run_snafu.py smallfile_wrapper/README.md smallfile_wrapper/smallfile_wrapper.py smallfile_wrapper/sync-diagram.png smallfile_wrapper/trigger_smallfile.py
$ echo "${diff_list}" 
ci/run_ci.sh
run_snafu.py
smallfile_wrapper/README.md
smallfile_wrapper/smallfile_wrapper.py
smallfile_wrapper/sync-diagram.png
smallfile_wrapper/trigger_smallfile.py

What do you think?

I like it. Good thinking @rsevilla87 . Let me update

@dry923 dry923 force-pushed the targeted_fix branch 2 times, most recently from 3d69a81 to f3520b3 Compare March 12, 2020 12:39
@rht-perf-ci
Copy link

Results for SNAFU CI Test

Test Result Runtime

@rht-perf-ci
Copy link

Results for SNAFU CI Test

Test Result Runtime
fio_wrapper PASS 00:15:42
fs_drift_wrapper PASS 00:09:37
hammerdb PASS 00:12:05
iperf PASS 00:06:50
pgbench-wrapper PASS 00:07:12
smallfile_wrapper PASS 00:10:31
sysbench PASS 00:05:01
uperf-wrapper PASS 00:23:51
ycsb-wrapper PASS 00:13:46

Copy link
Member

@rsevilla87 rsevilla87 left a comment

Choose a reason for hiding this comment

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

LGTM!

@aakarshg aakarshg merged commit 58e1ac9 into cloud-bulldozer:master Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok to test Kick off our CI framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants