JS Coverage

[AmitJoki]

Adds JS coverage for Feature Specs.

After the tests are run, the report can be viewed by running the server rails s and visiting localhost:3000/js_coverage/jscoverage.html

The workings are explained in comments.

[AmitJoki]

@ragesoss this is complete. The failing error is from prop type validation and it is indeed undefined. The other builds are passing because the production builds ignore prop validation.

[ragesoss]

I assume these schema.rb changes are unrelated noise.

[AmitJoki]

yes! This schema change always creeps up in my git diff everytime which is annoying. I'll revert this.

[ragesoss]

Looks like that's a bad prop type configuration, as I'm pretty sure (as exercised in the test) it's now the intended behavior to have ArticleViewer work with Wikipedia articles that don't have an Article record in the dashboard database, in the context of ArticleFinder. I guess just removing the isRequired is the quickest fix.

Can you add a summary of the coverage results in the console? Otherwise, it doesn't do much good to run the coverage in travis-ci.

[ragesoss]

After trying this out a little bit, I think the results showing up based on the concatenated webpack bundles rather than separately for each source file is the biggest limitation. This will make it too tedious to use the coverage stats as a quick way to identify frontend components that aren't being exercised in the feature specs (and too difficult to measure progress).

Any ideas for getting closer to the kind of output we get from the jest suite?

[AmitJoki]

I think if we disable vendors for test environment and then split the files manually and insert them instead of using main.js, we could have the report of individual files.

Shall I try this approach?

[ragesoss]

Does 'split the files manually' mean an entry in webpack config for every single source file? I guess if you can do it without a huge amount of work, it would be worth it just to get a snapshot of the main holes in coverage, even if it doesn't make sense to merge it that way.

[AmitJoki]

@ragesoss I got the splitting the files into its own entry points working, but hit up on another snag. Basically Webpack just goes through all the imports and dumps them in its own bundle.

So, a routes.jsx or an App.jsx which touches most of the code gets the same size as we would have gotten without the splitting of the files.

I have asked a question on Stack Overflow - https://stackoverflow.com/questions/63304864/webpack-how-to-ignore-the-imported-files to see if there's a way to not include the import ... from '...' files.

[AmitJoki]

@ragesoss can this be merged? I have added documents and cleaned up some code. The failures are probably due to some server error as they are passing on my local machine. If you can start this build again, it should pass.

[ragesoss]

I'm still uncomfortable with a few things about this, which I think are fixable. In broad strokes, I don't think the implementation is isolated enough as an optional and test-only feature. In particular:

• the behavior when running individual specs, which by default should run on the uninstrumented version of the assets... perhaps there should be a specific command to run tests with the JS coverage enabled, so that it never happens by accident.
• the runtime behavior change based on ENV['feature']... I think it would be better (for example) to add some test helper code that replaces the main version of hot_javascript_tag, instead of having the coverage code path in the production code. Same idea for main.js, although I'm not sure what a good solution is there.
• the code in rails_helper.rb for collecting test coverage should ideally be all pulled into a separate file and then required (maybe conditionally, based on whether coverage is enabled for this test run). Currently, the coverage code is intermingled with other test concerns in ways that will be confusing to isolate and maintain.

[AmitJoki]

@ragesoss I have made the following changes:

• Coverage is only enabled if COVERAGE=true.
• There's no pollution in production code.
• Coverage related spec helpers are put in its own file.
• There's no need to perform any additional steps for getting reports for individual tests now as those are derived from ARGV