Check MANIFEST when PR is created

[MichaelTimbert]

Purpose

Run 'make distcheck' in tests.
I followed what was done in "zonemaster/zonemaster-engine#903"
except that I added "t/manifest.t" file in MANIFEST.SKIP otherwise the installation of tar.gz fails.

Context

Fixes #119

How to test this PR

Make sure we don't interfere with the release process:

git clean -dfx   # Start out with a clean repository
perl Makefile.PL # Generate the top-level Makefile
make all         # Generate MO files and modules.txt
make distcheck   # Verify that distcheck doesn't report any missing or unrecognized files

Run the unit tests in the source directory:

make distcheck     # Make sure distcheck doesn't report any missing or unrecognized files
prove t/manifest.t # Verify the new test passes
touch new.file  # Create a file but don't add it to MANIFEST
prove t/manifest.t # Verify the new test fails
echo "new.file" >> MANIFEST.SKIP 
prove t/manifest.t # Verify the new test passes

Run the unit tests in the dist file:

make distcheck                               # Make sure your repo is clean
make dist                                    # Create the dist file
cpanm --test-only Zonemaster-LDNS-*.tar.gz # Run the tests included in the dist file

[matsduf]

The only other repository that has t/manifest.t is Zonemaster-Engine (I think that -CLI and -Backend also should have it). I made a comparison between this t/manifest.t and the one from Zonemaster-Engine and they are not 100% identical. Is there any reason for that?

Here t/manifest.t is in the MANIFEST.SKIP file, which seams correct, but in Zonemaster-Engine it is in MANIFEST.

I think we should have the same handling in all the repositories, if possible.

[MichaelTimbert]

The only other repository that has t/manifest.t is Zonemaster-Engine (I think that -CLI and -Backend also should have it). I made a comparison between this t/manifest.t and the one from Zonemaster-Engine and they are not 100% identical. Is there any reason for that?

No, i have updated 't/manifest.t" to be the same as the on on Zonemaster-Engine.

[matsduf]

Run the unit tests in the dist file:

make distcheck                               # Make sure your repo is clean
make dist                                    # Create the dist file
cpanm --test-only Zonemaster-LDNS-*.tar.gz # Run the tests included in the dist file

Since t/manifest.t is not in MANIFEST it will not be included in the dist file, and nothing special will happen with cpanm --test-only. It is more relevant to see what how Travis handles updated and not updated MANIFEST and MANIFEST.SKIP files.

[matsduf]

Works as expected in Travis.

[matsduf]

@MichaelTimbert, do you plan to update Zonemaster-Engine and copy this PR to Zonemaster-CLI and Zonemaster-Backend to make all have the same behavior?

[MichaelTimbert]

I'm not sure about this solution, I read @mattias-p's comments on zonemaster/zonemaster-engine#903 and agree that this is more like a hack than a correct solution.
To me /t contains functional tests and that's what people expect. Adding the MANIFEST check is more of a Continuous Delivery(CD) test because it check the tar.gz file and not the code itself.
This is why i think it preferable to test MANIFEST* in github action (as proposed in #201), we can separate CI and CD by checking MANIFEST in another github-action's job for more readability. But in this case we will be more dependent of Github for testing.

[matsduf]

This is why i think it preferable to test MANIFEST* in github action (as proposed in #201), we can separate CI and CD by checking MANIFEST in another github-action's job for more readability. But in this case we will be more dependent of Github for testing.

We host Zonemaster on Github. Is the dependency on Github for CI work really a problem?

[MichaelTimbert]

We host Zonemaster on Github. Is the dependency on Github for CI work really a problem?

No, not really , it is just something to get in mind while making this choice.

[MichaelTimbert]

@MichaelTimbert, do you plan to update Zonemaster-Engine and copy this PR to Zonemaster-CLI and Zonemaster-Backend to make all have the same behavior?

Yes i will do it :)

The base branch was changed.