Skip to content

Commit

Permalink
CI tweaks (#7069)
Browse files Browse the repository at this point in the history
- Run Checkstyle with gradle to make it easier for users
  - No longer needs different configuration for new code
  - Exclude current violations
  - Fix some violations that somehow couldn't be specified in the exclusion file
- Print SpotBugs/Lint/Checkstly violations in GitHub format
  - Then the CI run gets annotated on the web UI
  • Loading branch information
ByteHamster committed Apr 7, 2024
1 parent fc40da2 commit e578f4c
Show file tree
Hide file tree
Showing 44 changed files with 501 additions and 625 deletions.
1 change: 1 addition & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
-->
- [ ] I have read the contribution guidelines: https://github.com/AntennaPod/AntennaPod/blob/develop/CONTRIBUTING.md#submit-a-pull-request
- [ ] I have performed a self-review of my code
- [ ] I have run the automated code checks using `./gradlew checkstyle spotbugsPlayDebug spotbugsDebug :app:lintPlayDebug`
- [ ] My code follows the style guidelines of the AntennaPod project: https://github.com/AntennaPod/AntennaPod/wiki/Code-style
- [ ] I have mentioned the corresponding issue and the relevant keyword (e.g., "Closes: #xy") in the description (see https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
- [ ] If it is a core feature, I have added automated tests
57 changes: 22 additions & 35 deletions .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,8 @@ on:
branches: [ master, develop ]

jobs:
code-style:
name: "Code Style"
runs-on: ubuntu-latest
timeout-minutes: 45
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Checkstyle
run: |
curl -s -L https://github.com/checkstyle/checkstyle/releases/download/checkstyle-10.3.1/checkstyle-10.3.1-all.jar > checkstyle.jar
find . -name "*\.java" | xargs java -Dconfig_loc=config/checkstyle -jar checkstyle.jar -c config/checkstyle/checkstyle.xml
- name: Find PR Base Commit
id: vars
run: |
git fetch origin develop
echo "branchBaseCommit=$(git merge-base origin/develop HEAD)" >> $GITHUB_OUTPUT
- name: Diff-Checkstyle
run: |
curl -s -L https://github.com/yangziwen/diff-check/releases/download/0.0.7/diff-checkstyle.jar > diff-checkstyle.jar
java -Dconfig_loc=config/checkstyle -jar diff-checkstyle.jar -c config/checkstyle/checkstyle-new-code.xml --git-dir . --base-rev ${{ steps.vars.outputs.branchBaseCommit }}
- name: XML of changed files
run: |
curl -s -L https://github.com/ByteHamster/android-xml-formatter/releases/download/1.1.0/android-xml-formatter.jar > android-xml-formatter.jar
git diff --name-only ${{ steps.vars.outputs.branchBaseCommit }} --diff-filter=AM | { grep "res/layout/" || true; } | xargs java -jar android-xml-formatter.jar
test $(git diff | wc -l) -eq 0 || (echo -e "\n\n===== Found XML code style violations! See output below how to fix them. =====\n\n" && git --no-pager diff --color=always && false)
wrapper-validation:
name: "Gradle Wrapper Validation"
needs: code-style
runs-on: ubuntu-latest
timeout-minutes: 45
steps:
Expand All @@ -45,11 +17,12 @@ jobs:

static-analysis:
name: "Static Code Analysis"
needs: code-style
runs-on: ubuntu-latest
timeout-minutes: 45
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Set up JDK 17
uses: actions/setup-java@v4
with:
Expand All @@ -62,14 +35,24 @@ jobs:
~/.gradle/caches
~/.gradle/wrapper
key: gradle-${{ hashFiles('**/*.gradle*') }}-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }}
- name: Lint :app module recursively
run: ./gradlew :app:lintPlayRelease
- name: SpotBugs
run: ./gradlew spotbugsPlayDebug spotbugsDebug
- name: Configure parallel build
run: echo "org.gradle.parallel=true" >> local.properties
- name: XML code style
run: |
curl -s -L https://github.com/ByteHamster/android-xml-formatter/releases/download/1.1.0/android-xml-formatter.jar > android-xml-formatter.jar
find . -wholename "*/res/layout/*.xml" | xargs java -jar android-xml-formatter.jar
test $(git diff | wc -l) -eq 0 || (echo -e "\n\n===== Found XML code style violations! See output below how to fix them. =====\n\n" && git --no-pager diff --color=always && false)
- name: Checkstyle, Lint, SpotBugs
run: ./gradlew checkstyle :app:lintPlayDebug spotbugsPlayDebug spotbugsDebug
- name: Generate readable error messages for GitHub
if: failure()
run: |
git diff --name-only | xargs -I '{}' echo "::error file={},line=1,endLine=1,title=XML Format::Run android-xml-formatter.jar on this file or view CI output to see how it should be formatted."
python .github/workflows/errorPrinter.py
unit-test:
name: "Unit Test: ${{ matrix.variant }}"
needs: code-style
needs: static-analysis
runs-on: ubuntu-latest
timeout-minutes: 45
strategy:
Expand Down Expand Up @@ -101,6 +84,8 @@ jobs:
~/.gradle/caches
~/.gradle/wrapper
key: gradle-${{ hashFiles('**/*.gradle*') }}-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }}
- name: Configure parallel build
run: echo "org.gradle.parallel=true" >> local.properties
- name: Create temporary release keystore
run: keytool -noprompt -genkey -v -keystore "app/keystore" -alias alias -storepass password -keypass password -keyalg RSA -validity 10 -dname "CN=antennapod.org, OU=dummy, O=dummy, L=dummy, S=dummy, C=US"
- name: Build
Expand All @@ -116,7 +101,7 @@ jobs:

emulator-test:
name: "Emulator Test"
needs: code-style
needs: static-analysis
runs-on: macOS-latest
timeout-minutes: 45
env:
Expand All @@ -135,6 +120,8 @@ jobs:
~/.gradle/caches
~/.gradle/wrapper
key: gradle-${{ hashFiles('**/*.gradle*') }}-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }}
- name: Configure parallel build
run: echo "org.gradle.parallel=true" >> local.properties
- name: Build with Gradle
run: ./gradlew assemblePlayDebugAndroidTest
- name: Android Emulator test
Expand Down
48 changes: 48 additions & 0 deletions .github/workflows/errorPrinter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#!/bin/python
from xml.dom import minidom
import os.path
import glob
from pathlib import Path

if os.path.isfile('app/build/reports/lint-results-playDebug.xml'):
dom = minidom.parse('app/build/reports/lint-results-playDebug.xml')
issues = dom.getElementsByTagName('issue')
for issue in issues:
locations = issue.getElementsByTagName('location')
for location in locations:
print(location.attributes['file'].value + ":" + location.attributes['line'].value + " " + issue.attributes['summary'].value)
print("::error file=" + location.attributes['file'].value
+ ",line=" + location.attributes['line'].value
+ ",endLine=" + location.attributes['line'].value
+ ",title=Lint::" + issue.attributes['summary'].value + ". " + issue.attributes['explanation'].value.replace('\n', ' '))
print()

if os.path.isfile('build/reports/checkstyle/checkstyle.xml'):
dom = minidom.parse('build/reports/checkstyle/checkstyle.xml')
files = dom.getElementsByTagName('file')
for f in files:
errors = f.getElementsByTagName('error')
for error in errors:
print(f.attributes['name'].value + ":" + error.attributes['line'].value + " " + error.attributes['message'].value)
print("::error file=" + f.attributes['name'].value
+ ",line=" + error.attributes['line'].value
+ ",endLine=" + error.attributes['line'].value
+ ",title=Checkstyle::" + error.attributes['message'].value)
print()


for filename in glob.iglob('**/build/reports/spotbugs/*.xml', recursive=True):
filenamePath = Path(filename)
dom = minidom.parse(filename)
instance = dom.getElementsByTagName('BugInstance')
for inst in instance:
lines = inst.getElementsByTagName('SourceLine')
longMessage = inst.getElementsByTagName('LongMessage')[0].firstChild.nodeValue
for line in lines:
if "primary" in line.attributes:
print(line.attributes['sourcepath'].value + ": " + longMessage)
print("::error file=" + str(filenamePath.parent.parent.parent.parent.absolute()) + "/src/main/java/" + line.attributes['sourcepath'].value
+ ",line=" + line.attributes['start'].value
+ ",endLine=" + line.attributes['end'].value
+ ",title=SpotBugs::" + longMessage.replace('\n', ' '))
print()
3 changes: 2 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ Submit a pull request
- Get coding :)
- If possible, add unit tests for your pull request and make sure that they pass.
- Please do not upgrade dependencies or build tools unless you have a good reason for it. Doing so can easily introduce bugs that are hard to track down.
- Please follow our code style. You can use Checkstyle within Android Studio using our [configuration file](https://github.com/AntennaPod/AntennaPod/blob/develop/config/checkstyle/checkstyle-new-code.xml).
- Please follow our code style. You can use Checkstyle within Android Studio using our [configuration file](https://github.com/AntennaPod/AntennaPod/blob/develop/config/checkstyle/checkstyle.xml).
- To check the code style locally, run `./gradlew checkstyle spotbugsPlayDebug spotbugsDebug :app:lintPlayDebug`
- Please only change the English string resources. Translations are handled on [Transifex](https://www.transifex.com/antennapod/antennapod/).
- Open the PR
- Mention the corresponding issue in the pull request text, so that it can be closed once your pull request has been merged. If you use [special keywords](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue), GitHub will close the issue(s) automatically.
Expand Down
10 changes: 4 additions & 6 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,11 @@ android {
}

lint {
disable 'ObsoleteLintCustomCheck', 'CheckResult', 'UnusedAttribute', 'BatteryLife', 'InflateParams',
'RestrictedApi', 'TrustAllX509TrustManager', 'ExportedReceiver', 'AllowBackup', 'VectorDrawableCompat',
'StaticFieldLeak', 'UseCompoundDrawables', 'NestedWeights', 'Overdraw', 'UselessParent', 'TextFields',
'AlwaysShowAction', 'Autofill', 'ClickableViewAccessibility', 'ContentDescription',
disable 'CheckResult', 'MissingMediaBrowserServiceIntentFilter', 'UnusedAttribute', 'InflateParams',
'RestrictedApi', 'ExportedReceiver', 'NotifyDataSetChanged', 'UseCompoundDrawables', 'NestedWeights',
'Overdraw', 'UselessParent', 'TextFields', 'AlwaysShowAction', 'Autofill', 'ClickableViewAccessibility',
'KeyboardInaccessibleWidget', 'LabelFor', 'SetTextI18n', 'HardcodedText', 'RelativeOverlap',
'RtlCompat', 'RtlHardcoded', 'MissingMediaBrowserServiceIntentFilter', 'VectorPath',
'InvalidPeriodicWorkRequestInterval', 'NotifyDataSetChanged', 'RtlEnabled'
'RtlHardcoded', 'RtlEnabled', 'ContentDescription'
}

androidResources {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
/**
* Utility methods for FeedGenerator
*/
class GeneratorUtil {
private GeneratorUtil(){}

abstract class GeneratorUtil {
public static void addPaymentLink(XmlSerializer xml, String paymentLink, boolean withNamespace) throws IOException {
String ns = (withNamespace) ? "http://www.w3.org/2005/Atom" : null;
xml.startTag(ns, "link");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,12 @@ private void setProxyConfig() {

private final TextWatcher requireTestOnChange = new TextWatcher() {
@Override
public void beforeTextChanged(CharSequence s, int start, int count, int after) {}
public void beforeTextChanged(CharSequence s, int start, int count, int after) {
}

@Override
public void onTextChanged(CharSequence s, int start, int before, int count) {}
public void onTextChanged(CharSequence s, int start, int before, int count) {
}

@Override
public void afterTextChanged(Editable s) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
public class LockableBottomSheetBehavior<V extends View> extends ViewPagerBottomSheetBehavior<V> {
private boolean isLocked = false;

public LockableBottomSheetBehavior() {}
public LockableBottomSheetBehavior() {
}

public LockableBottomSheetBehavior(Context context, AttributeSet attrs) {
super(context, attrs);
Expand Down
4 changes: 2 additions & 2 deletions app/src/main/res/layout/alternate_urls_dropdown_item.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<CheckedTextView
xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@android:id/text1"
style="?android:attr/spinnerDropDownItemStyle"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:paddingVertical="8dp" />
android:paddingVertical="8dp"
style="?android:attr/spinnerDropDownItemStyle" />
4 changes: 2 additions & 2 deletions app/src/main/res/layout/alternate_urls_item.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
<TextView
xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@android:id/text1"
style="?android:attr/spinnerItemStyle"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:maxLines="3"
android:textAlignment="inherit" />
android:textAlignment="inherit"
style="?android:attr/spinnerItemStyle" />
42 changes: 22 additions & 20 deletions app/src/main/res/layout/bug_report.xml
Original file line number Diff line number Diff line change
@@ -1,28 +1,30 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:orientation="vertical"
android:padding="16dp">
<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:orientation="vertical"
android:padding="16dp">

<Button
android:id="@+id/btn_open_bug_tracker"
android:text="@string/open_bug_tracker"
android:layout_width="match_parent"
android:layout_height="wrap_content"/>
android:id="@+id/btn_open_bug_tracker"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:text="@string/open_bug_tracker" />

<Button
android:id="@+id/btn_copy_log"
android:text="@string/copy_to_clipboard"
android:layout_width="match_parent"
android:layout_height="wrap_content"/>
android:id="@+id/btn_copy_log"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:text="@string/copy_to_clipboard" />

<TextView
android:layout_marginTop="8dp"
android:id="@+id/crash_report_logs"
android:textIsSelectable="true"
android:textSize="12sp"
android:layout_width="match_parent"
android:layout_height="0dp"
android:layout_weight="1"/>
android:id="@+id/crash_report_logs"
android:layout_width="match_parent"
android:layout_height="0dp"
android:layout_marginTop="8dp"
android:textIsSelectable="true"
android:textSize="12sp"
android:layout_weight="1" />

</LinearLayout>
27 changes: 14 additions & 13 deletions app/src/main/res/layout/checkbox_do_not_show_again.xml
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="vertical"
android:paddingLeft="16dp"
android:paddingRight="16dp"
android:paddingTop="8dp"
android:paddingBottom="8dp">
<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="vertical"
android:paddingLeft="16dp"
android:paddingRight="16dp"
android:paddingTop="8dp"
android:paddingBottom="8dp">

<CheckBox
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:id="@+id/checkbox_do_not_show_again"
android:text="@string/checkbox_do_not_show_again"/>
android:id="@+id/checkbox_do_not_show_again"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:text="@string/checkbox_do_not_show_again" />

</LinearLayout>
</LinearLayout>
21 changes: 11 additions & 10 deletions app/src/main/res/layout/edit_text_dialog.xml
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:orientation="vertical"
android:padding="16dp">
<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:orientation="vertical"
android:padding="16dp">

<EditText
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:inputType="text"
android:ems="10"
android:id="@+id/urlEditText" />
android:id="@+id/urlEditText"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:inputType="text"
android:ems="10" />

</LinearLayout>
6 changes: 3 additions & 3 deletions app/src/main/res/layout/ellipsize_start_listitem.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent"
android:layout_height="match_parent"
Expand All @@ -19,4 +19,4 @@
tools:background="@android:color/holo_green_dark"
tools:text="List item title" />

</LinearLayout>
</LinearLayout>
Loading

0 comments on commit e578f4c

Please sign in to comment.