Skip to content

Commit

Permalink
Add the feature flag checks file to CI and fix failing issues
Browse files Browse the repository at this point in the history
  • Loading branch information
kennethmurerwa committed Jul 17, 2024
1 parent d7472c9 commit 51821b6
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 23 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/static_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ jobs:
run: |
bash /home/runner/work/oppia-android/oppia-android/scripts/ktlint_lint_check.sh $HOME
- name: Feature flag checks
run: |
bash /home/runner/work/oppia-android/oppia-android/scripts/feature_flags_check.sh $HOME
- name: Protobuf lint check
run: |
bash /home/runner/work/oppia-android/oppia-android/scripts/buf_lint_check.sh $HOME
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import org.oppia.android.util.platformparameter.PlatformParameterValue
import org.oppia.android.util.platformparameter.SPOTLIGHT_UI
import javax.inject.Inject
import javax.inject.Singleton
import org.oppia.android.util.platformparameter.ENABLE_MULTIPLE_CLASSROOMS
import org.oppia.android.util.platformparameter.EnableMultipleClassrooms

/**
* Convenience logger for feature flags.
Expand Down Expand Up @@ -64,7 +66,9 @@ class FeatureFlagsLogger @Inject constructor(
@EnableNpsSurvey
private val enableNpsSurvey: PlatformParameterValue<Boolean>,
@EnableOnboardingFlowV2
private val enableOnboardingFlowV2: PlatformParameterValue<Boolean>
private val enableOnboardingFlowV2: PlatformParameterValue<Boolean>,
@EnableMultipleClassrooms
private val enableMultipleClassrooms: PlatformParameterValue<Boolean>,
) {
/**
* A variable containing a list of all the feature flags in the app.
Expand All @@ -83,7 +87,8 @@ class FeatureFlagsLogger @Inject constructor(
INTERACTION_CONFIG_CHANGE_STATE_RETENTION to enableInteractionConfigChangeStateRetention,
APP_AND_OS_DEPRECATION to enableAppAndOsDeprecation,
ENABLE_NPS_SURVEY to enableNpsSurvey,
ENABLE_ONBOARDING_FLOW_V2 to enableOnboardingFlowV2
ENABLE_ONBOARDING_FLOW_V2 to enableOnboardingFlowV2,
ENABLE_MULTIPLE_CLASSROOMS to enableMultipleClassrooms
)

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash

echo "********************************"
echo "Performing feature flag checks"
echo "Running feature flag checks"
echo "********************************"

failed_checks=0
Expand Down Expand Up @@ -59,6 +59,11 @@ function get_imported_classes_in_logger() {
# Get annotation classes
logged_classes=$(grep -E '@Enable[A-Za-z0-9_]+' "$file_path")

# Replace the @ symbol and spaces within each element of the list
for i in "${!logged_classes[@]}"; do
logged_classes[$i]=$(echo "${logged_classes[$i]}" | tr -d '@' | tr -d ' ')
done

# Print the logged classes
echo "$logged_classes"
}
Expand Down Expand Up @@ -107,38 +112,32 @@ function ensure_all_flags_are_imported() {
imported_classes=($(get_imported_classes_in_logger))
flags_added_to_map=($(get_flags_added_into_the_logging_map))

# Replace the @ symbol and spaces within each element of the list
for i in "${!imported_classes[@]}"; do
imported_classes[$i]=$(echo "${imported_classes[$i]}" | tr -d '@' | tr -d ' ')
done
file_path=$(find . -name FeatureFlagsLogger.kt)
imports_line_number=$(grep -n 'class FeatureFlagsLogger @Inject constructor(' "$file_path" | head -n 1 | awk -F: '{print $1}')
flags_map_line_number=$(grep -n 'Map<String, PlatformParameterValue<Boolean>> = mapOf(' "$file_path" | head -n 1 | awk -F: '{print $1}')

# Iterate through each element in the feature_flags array to check if it is imported
# in the FeatureFlagsLogger.kt
for element in "${feature_flags[@]}"; do
# Check if the element is in the imported_classes array
in_array=$(item_in_array "$element" "${imported_classes[@]}")
if [[ $in_array -ne 1 ]]; then
failed_checks=$((failed_checks + 1))
echo "ERROR: $element is not imported in FeatureFlagsLogger.kt"
echo "$element is not imported in the constructor argument in $file_path at line $imports_line_number"
fi
done

# Iterate through each element in the feature_flags array to check if it is added to the logging map
for element in "${feature_flags[@]}"; do
# Check if the element is in the feature_flags array
in_array=$(item_in_array "$element" "${flags_added_to_map[@]}")
if [[ $in_array -ne 1 ]]; then
failed_checks=$((failed_checks + 1))
echo "ERROR: $element is not added to the logging map in FeatureFlagsLogger.kt"
echo "$element is not added to the logging map in $file_path at line $flags_map_line_number"
fi
done

if [[ $failed_checks -eq 0 ]]; then
echo "All feature flags are imported and added to the logging map in FeatureFlagsLogger.kt"
echo "Feature flag checks completed successfully"
exit 0
else
echo "********************************"
echo "$failed_checks Feature flag issues found."
echo "Feature flag issues found."
echo "Please fix the above issues."
echo "********************************"
exit 1
Expand Down
4 changes: 4 additions & 0 deletions scripts/static_checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ echo ""
bash scripts/ktlint_lint_check.sh
echo ""

# Run feature flag checks
bash scripts/feature_flags_check.sh
echo ""

# Run protobuf lint checks
bash scripts/buf_lint_check.sh
echo ""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,6 @@ const val DOWNLOADS_SUPPORT = "android_enable_downloads_support"
/** Default value for feature flag corresponding to [EnableDownloadsSupport]. */
const val ENABLE_DOWNLOADS_SUPPORT_DEFAULT_VALUE = false

/** Qualifier for the feature flag corresponding to enabling the language selection UI. */
@Qualifier
annotation class EnableLanguageSelectionUi

/** Default value for the feature flag corresponding to [EnableLanguageSelectionUi]. */
const val ENABLE_LANGUAGE_SELECTION_UI_DEFAULT_VALUE = true

/**
* Qualifier for the feature flag corresponding to enabling the extra topic tabs: practice and info.
*/
Expand Down

0 comments on commit 51821b6

Please sign in to comment.