Skip to content
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

Get master passing again #779

Closed
wants to merge 4 commits into from
Closed

Get master passing again #779

wants to merge 4 commits into from

Conversation

BookOfGreg
Copy link
Member

@BookOfGreg BookOfGreg commented Sep 5, 2017

Master branch does not pass travis on it's own, making it difficult to tell when new branches are complete.
I plan on cleaning up the appraisal setup with the aim of making this slightly easier to work on.

  • Handle the Rails 5.1.0.rc1 compatibility
  • Handle turbolinks better
  • Fix broken test on BUNDLE_GEMFILE=/Users/e0052727/source/react-rails/gemfiles/rails_3.2.gemfile bundle exec rake test at test_it_reloads_when_new_jsx_files_are_added_to_the_asset_pipeline Seems to be a flakey test, passes sometimes?
  • Consistently name tests from test "foo bar" do to def test_foo_bar
  • Rubocop with the standard ruby style
  • Fix broken test on rails 5.0 + webpacker 1.2 around manifest_path

@swrobel
Copy link

swrobel commented Sep 6, 2017

Why not use the test keyword? Rails still recommends it in their edgeguides and says the following: "Although you can still use regular method definitions, using the test macro allows for a more readable test name."

@BookOfGreg
Copy link
Member Author

BookOfGreg commented Sep 7, 2017

When a test fails with "FAILED: ControllerTest#test_something_happened", you can global find it if the test name matches test_something_happened but not test "some-thin'g ()happened".

Perhaps this is personal preference but it's much much faster for me to fix tests if I can find them.

Edit: Also when you pass it on the CLI with --name, it also takes it in the form TESTOPTS="--name test_something_happened"

@BookOfGreg
Copy link
Member Author

Think I found the cause of some of my remaining test failures. I'd been running Webpacker 1 against Webpack 2, which is why all my test files keep ending up in the wrong positions. Will probably implement a version check like https://github.com/shakacode/react_on_rails/blob/master/lib/react_on_rails/version_checker.rb to make at least the tests a bit more stable.

I'm a little concerned at how much work was involved at getting the master branch passing so far though. Am I doing something wrong or missing something obvious?

@rmosolgo
Copy link
Member

rmosolgo commented Sep 8, 2017

@BookOfGreg Thanks digging in on this!

how much work

I'm not surprised 😬 It was a lot of work to get webpacker working in the first place, and since then, the Webpacker APIs have changed a lot, so I bet it will be hard to get things working properly again 😖

@BookOfGreg
Copy link
Member Author

FYI The last fail on travis is in the webpacker:compile Rake task which is in capture_io, hence why there is no error output.
It's complaining about looking for the manifest in packs/packs when webpack put it in public/packs.
Honestly not sure what to do about that.

@BookOfGreg
Copy link
Member Author

Closing in favour of #777

@BookOfGreg BookOfGreg closed this Sep 12, 2017
@BookOfGreg BookOfGreg deleted the update-appraisal branch September 12, 2017 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants