Validate package.json in all tracks

[tejasbubane]

We are relying on copying package.json in all exercises. It is difficult to check all those file changes in the PR eg. #286 has 68 file changes. Although the copying is automated using Makefile, I think we should be extra-cautious since these files are directly delivered to users.

We can add some CI check similar to configlet but for our numerous package.json files. We can check for json structure and that all files are identical. This will make sure someone did not change anything unexpected in the PR.

This of-course depends on #235. If we are unable to find any way to remove those numerous package.json files, we should make sure to keep them valid and checked in CI.

[rchavarria]

Sounds interesting but I'm not sure it'll be useful.

Everytime a maintainer run a build locally, all package.json file are copied again. So, if a contributor (or other maintainer) makes a change in a package.json for a single file, the maintainer's action should detect that change, and could talk to the contributor to fix that single change.

@exercism/javascript Do you find this action (run a build locally) enough for detecting incomplete changes on package.json files?

[joeljuca]

I'm with @tejasbubane. We should check those files in CI to make sure they are the same in all repos.

Right now, almost all files are equal - except one:

❯ find . -name package.json | xargs md5 -q | uniq -c
  37 61ca18ec644e8c9170124e986ed3068d
   1 bdca6a9a842cee44c258e2b27768ea77
  30 61ca18ec644e8c9170124e986ed3068d

The one with a change is the ./exercises/package.json:

❯ find . -name package.json | xargs md5 | grep bdca6a9a842cee44c258e2b27768ea77
MD5 (./exercises/package.json) = bdca6a9a842cee44c258e2b27768ea77

Which has one additional npm script:

❯ diff ./package.json ./exercises/package.json
29d28
<     "watch": "jest --no-cache --watch ./*",

So, we are already suffering for not having an automated check in CI. We should checksum all package.json files and see if their MD5s are the same as the ./package.json one.

[rchavarria]

First of all, that first find command is impressive. That will be an excellent first step to verify package.json files.

I'm a bit lost about the code right now. But, is ./exercises/package.json file needed? ./package.json is the file that is copied to all single exercises (./exercisses/<exercise>/package.json). Is ./exercises/package.json needed by Jest?

[tejasbubane]

@rchavarria

Is ./exercises/package.json needed by Jest?

It has babel and eslint config and most importantly the required packages to install (which cannot be installed globally, atleast not all of them). Jest works with babel-jest to transpile ecmascript on the fly while running the tests. I am not sure this plugin can be installed globally. Without this plugin, Jest won't be able to understand latest ecmascript features.

I am not sure if all of this would work without the package.json, but even if it did, it would be unconventional IMO since package.json is standard in all projects with some kind of build/transpile process.

Everytime a maintainer run a build locally, all package.json file are copied again. So, if a contributor (or other maintainer) makes a change in a package.json for a single file, the maintainer's action should detect that change, and could talk to the contributor to fix that single change.

Do you mean you run build locally for each PR merged?
What if my PR eg. #281 has a missing , in package.json of one of the exercises which goes unnoticed in the PR (because there are changes to all package.json files which all look same) - the PR is merged in - Katrina updates trackler and that package.json file with invalid json gets served to users? Even though travis runs make test, that file with error will never be corrected and commited back to the repo.
Correct me if I am wrong here.

[matthewmorgan]

Hey guys, package.json is needed by Jest. There's a configuration that tells it which directories to target to search for *spec.js files. For any given exercise, that should only be that exercise's tests.

It sounds to me like @joelwallis is suggesting that we do the check as part of the CI process-- I think that's a great idea. We have two cases to cover:

1. Allow the contributor to run the Makefile, so that it copies the global package.json to each directory
2. Have CI check to make sure all the individual files match the global one before doing the copy.

Ideas? It feels like we have a chicken-and-egg problem here, and that maybe the best way to resolve it is to know if we're running on CI or locally.

[rchavarria]

Yeah! I know package.json file is mandatory, I think @tejasbubane missunderstood me. We have package.json in three different places:

1. The root folder, ./package.json. I think this is the main one, the one that is copied to all exercises, the only truth.
2. Each exercise folder, ./exercises/hello_world/package.json, ./exercises/foobar/package.json,... These are copied from the package.json in the root folder.
3. The last one, the one I'm asking if we need it. It's on ./exercises. It isn't in any specific exercise. And it's the only one that is different. So, it's not copied from the main one, the one in the root folder. My question was if we need it. Maybe, it's created automatically by Jest. If we need it, I'm ok, we have to keep it. No prob.

---

About the checking script: I see where @matthewmorgan is going. Yep, if we check all package.json, how could we update them? If we copy before checking, how could we know they are ok?

From my point of view, I only review the main one (./package.json). Others must be a copy of it. I usually run make (or make test) locally, on a PR, to see if all tests pass in my machine. If it gets some files modified, something is not completely done in the PR.

When I talk about the build process, I mean I run the make command locally. CI should run the same command. CI should get a clean copy of the code, and run npm install and make (or make test). make command should take care of all the build process: iterate over all exercises, transpile them (ok, we don't need this step with Jest), enable specs (replace xit with it), copy ./package.json file,...

One of those steps could be to check whatever we want on ./package.json. If the check is done before the copy, all package.json should be ok.

[joeljuca]

@rchavarria me too. I can't see where exercises/package.json is necessary.

About validating package.json files, we can just find them all and hash with MD5. All package.json inside exercises' folders must have the same output of the package.json in the project's root directory.