Test files ship with graphql-pro and graphql-enterprise

[mroach]

Currently the graphql gem doesn't ship with any test files, but graphql-pro and graphql-enterprise do. The former makes good sense to me as I don't see why a gem would ship with tests or any other code not necessary for the host applications.

I assume pro and enterprise are using default gemspec behaviour which aggressively includes all files in the shipped gem rather than this being an explicit choice.

I've always had a release build step to delete dirs like test, spec, examples, docs from installed gems, and in light of what's gone on with xz, I'm looking to see if there's any reason why gems ship with test files at all, and if not, help out with changing gemspec files to omit them. Since I can't open a PR for pro or enterprise, I'm creating an issue for discussion.

[rmosolgo]

Hey, thanks for raising this issue. I'm open to omitting them.

I originally included them because I read some where (don't remember where...) that a gem should include its tests as a kind of documentation or debugging aid for developers who download it. In practice, I doubt that ever happens, and as you noticed, GraphQL-Ruby doesn't include them. So I'm game to remove them. (Although the difference with GraphQL-Ruby is that the tests are available from GitHub...)

I'll update the gemspecs soon and follow up here when new releases are available.

[mroach]

I was also looking around for what the community opinion is with including tests and docs in gemfiles or not and stumbled across this discussion: https://github.com/orgs/rubygems/discussions/7551

I see the merit in including tests and docs for users that access them via the installed gem bundle. It's not a point I had considered since I don't personally do that (I also do wonder how many people do this). Although in the case of pro and enterprise I do sometimes dig into the gem source to figure something out. Where I think the structure of gems themselves fall-short is not having a way to strip out files not required for runtime. It already has this for runtime vs development dependencies and IMO should have the same for files.

Zero pressure from me on what to do here. I hadn't seen the other side of this argument until after I opened this issue. I appreciate the engagement here :) Feel free to close!

[rmosolgo]

Thanks for sharing that. Thinking through it, could you elaborate a bit on this:

what's gone on with xz

Reading up on xz, I'm not totally clear how it's connected here. Does that vulnerability play into the decision here?

[mroach]

It's only sort of related. The malicious code that was selectively injected into the built versions of liblzma was hidden in test files. The build process would decide at build time if it wanted to install the backdoor, and if so, extract obfuscated data from binary files stored as sample fodder for tests. This got me thinking again about the practice of shipping test files to production.

My general principle is to for an application to have the smallest and simplest footprint possible to still run. So no unnecessary system software or dependencies, then run it as non-root with a readonly filesystem, restrict outbound network activity, etc. All software and dependencies are liabilities, so I don't want any liabilities that don't serve a purpose.

Including such files also increases the footprint of code to audit. I don't always do it, but I do make some effort to scan the diffs of dependencies before upgrading. Before now I never thought to also check test files, but there's not much stopping a piece of code from loading a test file at runtime.

I also use a vulnerability scanner on our release Docker images and get hits on references to outdated/insecure gems caused by Gemfile.lock or .gemspec being included in some gems. Granted these are false positives, but I don't want to condition myself or anyone to ignore vulnerability scan hits as noise lest a real problem be shrugged off. (None of the graphql-ruby gems are included here :))

Anyway, it doesn't seem like there's community consensus on including test files or not and there are apparently users that like it as-is, so I'll just keep deleting them as part of my build process.

[rmosolgo]

Thanks for sharing your thoughts here. With a better understanding of the situation and tradeoffs, I'm planning to keep distributing the test files with the source.