Skip to content

Fix file duplication issue - YAML validator now skips JSON files when not in yamlAsJson mode #91

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

Merged
merged 4 commits into from
May 29, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented May 29, 2025

Problem

When using the files input to specify both JSON and YAML files, the action was processing files twice and reporting incorrect file counts. For example:

files: |
  moonstone-configuration.json
  manifests-configuration.yaml

Would produce output like:

moonstone-configuration.json is valid
moonstone-configuration.json is valid  # ❌ Duplicate processing
manifests-configuration.yaml is valid
✅ all 1 detected JSON files are valid
✅ all 2 detected YAML files are valid  # ❌ Wrong count

Root Cause

The YAML validator was missing logic to skip JSON files when yamlAsJson is false. While the JSON validator correctly skipped YAML files, the YAML validator processed all files from the files input regardless of their extension, leading to duplicate processing of JSON files.

Solution

Added symmetrical logic to the YAML validator to skip JSON files when not in yamlAsJson mode:

  1. Added jsonExtension input parameter to the YAML validator
  2. Added JSON file detection logic similar to how the JSON validator detects YAML files
  3. Added debug logging for transparency when JSON files are skipped
  4. Added test coverage to ensure the fix works correctly

After Fix

The same configuration now produces correct output:

moonstone-configuration.json is valid  # ✅ Processed by JSON validator only
manifests-configuration.yaml is valid  # ✅ Processed by YAML validator only
✅ all 1 detected JSON files are valid  # ✅ Correct count
✅ all 1 detected YAML files are valid  # ✅ Correct count

Changes Made

  • src/functions/yaml-validator.js: Added logic to detect and skip JSON files when yamlAsJson is false
  • tests/functions/yaml-validator.test.js: Added missing environment variable and test case for JSON file skipping

Verification

  • ✅ All existing tests pass (95/95)
  • ✅ 100% test coverage maintained
  • ✅ New test specifically validates JSON file skipping behavior
  • ✅ Code formatting and linting passes
  • ✅ Changes bundled for production

Fixes #70.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • img.shields.io
    • Triggering command: node /home/REDACTED/work/json-yaml-validate/json-yaml-validate/node_modules/.bin/make-coverage-badge --output-path ./badges/coverage.svg (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@Copilot Copilot AI changed the title [WIP] Bug: Wrong file extension in summary and double check of same file Fix file duplication issue - YAML validator now skips JSON files when not in yamlAsJson mode May 29, 2025
@Copilot Copilot AI requested a review from GrantBirki May 29, 2025 05:46
Copilot finished work on behalf of GrantBirki May 29, 2025 05:46
@GrantBirki GrantBirki marked this pull request as ready for review May 29, 2025 06:00
@GrantBirki GrantBirki added the bug Something isn't working label May 29, 2025
@GrantBirki GrantBirki merged commit 250fa0d into main May 29, 2025
4 checks passed
@GrantBirki GrantBirki deleted the copilot/fix-70 branch May 29, 2025 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Wrong file extension in summary and double check of same file
2 participants