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.

Sign up for GitHub

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

Issue #529: Deprecate launch.groovy and merge functionality into diff.groovy #546

Merged
merged 1 commit into from
Apr 10, 2021
Merged

Issue #529: Deprecate launch.groovy and merge functionality into diff.groovy #546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion .ci/travis.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ patch-diff-report-tool)
checkstyle-tester-launch-groovy)
checkout_from https://github.com/checkstyle/checkstyle
cd .ci-temp/checkstyle
LOCAL_GIT_REPO=$(pwd)
mvn --batch-mode clean install -Passembly
cd ../../checkstyle-tester
groovy launch.groovy -l projects-for-travis.properties -c my_check.xml -i
groovy diff.groovy -r "$LOCAL_GIT_REPO" -l projects-for-travis.properties --patchConfig my_check.xml \
--patchBranch master --mode single --allowExcludes
;;

checkstyle-tester-diff-groovy-patch)
Expand Down Expand Up @@ -69,6 +71,37 @@ checkstyle-tester-diff-groovy-patch-only)
-pc my_check.xml -p patch-branch -r ../.ci-temp/checkstyle -m single
;;

checkstyle-tester-diff-groovy-regression-single)
Copy link
Member

Choose a reason for hiding this comment

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

The way i understand this, you run regression using Contribution Master and Contribution PR and check if the 2 separate runs produce the same or different files. I assume difference will fail the CI (and are we sure of that?).

If the above is correct, is this planned to be merged in? What happens if we make diff-report-util change the layout. This will then always fail CI even though it is an expected change.

Copy link
Member Author

Choose a reason for hiding this comment

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

I assume difference will fail the CI (and are we sure of that?).

Yes. https://travis-ci.org/github/checkstyle/contribution/jobs/763804426

If the above is correct, is this planned to be merged in? What happens if we make diff-report-util change the layout. This will then always fail CI even though it is an expected change.

I would guess at that point we would need to use some sort of more complicated awk or perl command to verify that violations match up; right now, this CI job confirms that both the violations and format of the report are consistent.

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't do anything too complicated. Should we not merge it or is there some alternative for this to not cause issues easily?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should keep these, I have pushed several times in this PR, and these jobs haven't been flaky. It is the best way that I can think of to easily check that the violations, reports, and report formats are all consistent.

I think it is ok to wait and see what the future brings regarding changes to patch-diff-util before doing anything different here.

# Check out lateset checkstyle from master
rm -rf .ci-temp
checkout_from https://github.com/checkstyle/checkstyle

# Run report from master branch of contribution
checkout_from https://github.com/checkstyle/contribution
cd .ci-temp/contribution/checkstyle-tester
sed -i'' 's/^guava/#guava/' projects-to-test-on.properties
sed -i'' 's/#checkstyle|/checkstyle|/' projects-to-test-on.properties
export MAVEN_OPTS="-Xmx2048m"
groovy ./diff.groovy --listOfProjects projects-to-test-on.properties \
-pc ../../../checkstyle-tester/diff-groovy-regression-config.xml \
rnveach marked this conversation as resolved.
Show resolved Hide resolved
-r ../../checkstyle -xm "-Dcheckstyle.failsOnError=false" \
-m single -p master

# Run report with current branch
cd ../../../checkstyle-tester/
sed -i'' 's/^guava/#guava/' projects-to-test-on.properties
sed -i'' 's/#checkstyle|/checkstyle|/' projects-to-test-on.properties
rm -rf reports repositories
groovy ./diff.groovy --listOfProjects projects-to-test-on.properties \
-pc diff-groovy-regression-config.xml -r ../.ci-temp/checkstyle/ \
-m single -p master -xm "-Dcheckstyle.failsOnError=false"

cd ..
# We need to ignore file paths below, since they will be different between reports
diff -I "contribution" checkstyle-tester/reports/diff/checkstyle/index.html \
.ci-temp/contribution/checkstyle-tester/reports/diff/checkstyle/index.html
;;

codenarc)
cd checkstyle-tester
./codenarc.sh . diff.groovy > diff.log && cat diff.log && grep '(p1=0; p2=0; p3=0)' diff.log
Expand Down
4 changes: 2 additions & 2 deletions appveyor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,15 @@ environment:
CMD3: " && cd checkstyle && git checkout -b patch-branch"
CMD4: " "
CMD5: " && cd ..\\checkstyle-tester "
CMD6: " && groovy diff.groovy -l projects-for-travis.properties -c my_check.xml -b master -p patch-branch -r C:\\projects\\contribution\\checkstyle -i -s"
CMD6: " && groovy diff.groovy -l projects-for-travis.properties -c my_check.xml -b master -p patch-branch -r C:\\projects\\contribution\\checkstyle -s"
- JAVA_HOME: C:\Program Files\Java\jdk1.8.0
DESC: "checkstyle-tester (diff.groovy with base and patch configs) on guava"
CMD1: " git clone -q --depth=10 --branch=master "
CMD2: " https://github.com/checkstyle/checkstyle C:\\projects\\contribution\\checkstyle "
CMD3: " && cd checkstyle && git checkout -b patch-branch"
CMD4: " "
CMD5: " && cd ..\\checkstyle-tester "
CMD6: " && groovy diff.groovy -l projects-for-travis.properties -bc my_check.xml -pc my_check.xml -b master -p patch-branch -r C:\\projects\\contribution\\checkstyle -i -s"
CMD6: " && groovy diff.groovy -l projects-for-travis.properties -bc my_check.xml -pc my_check.xml -b master -p patch-branch -r C:\\projects\\contribution\\checkstyle -s"

build_script:
- ps: >
Expand Down
2 changes: 2 additions & 0 deletions checkstyle-tester/LAUNCH_GROOVY_README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# _LAUNCH.GROOVY IS BEING DEPRECATED!!!_

# launch.groovy

Checkstyle report generation
Expand Down
8 changes: 6 additions & 2 deletions checkstyle-tester/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ you may require other tools like Git or Mericural, for Git and HG repositories r

`diff.groovy` supports the following command line arguments:

**localGitRepo** (r) - path to the local Checkstyle repository (required);
**localGitRepo** (r) - path to the local Checkstyle repository (required in diff mode);

**baseBranch** (b) - name of the base branch in local Checkstyle repository
(optional, if absent, then the tool will use only patchBranch in case the tool
Expand All @@ -42,7 +42,7 @@ will finish the execution with the error.
You must specify 'patchBranch' and 'patchConfig' if the mode is 'single', and 'baseBranch',
'baseConfig', 'patchBranch', and 'patchConfig' if the mode is 'diff');

**patchBranch** (p) - name of the branch with your changes (required);
**patchBranch** (p) - name of the branch with your changes (required in diff mode);

**baseConfig** (bc) - path to the base checkstyle configuration file.
It will be applied to base branch (required if patchConfig is specified);
Expand All @@ -65,6 +65,10 @@ This option is useful for Windows users where they are restricted to maximum dir
to maven during the Checkstyle regression run. For example, if you want to skip site generation, you
would add `--extraMvnRegressionOptions "-Dmaven.site.skip=true"` to `diff.groovy` execution.

**allowExcludes** (g) - this option tells `diff.groovy` to allow paths and files defined in the file specified for
the `--listOfProjects (-l)` argument to be excluded from report generation (optional, default is false). This option is
used for files that are not compilable or that Checkstyle cannot parse.

## Outputs

When the script finishes its work the following directory structure will be created
Expand Down
Loading