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

When route not found during testing, don't log error #615

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lorensr
Copy link

@lorensr lorensr commented May 4, 2016

meteor/todos is currently throwing this error on the client during meteor test.

meteor/todos#130 (comment)

@bcbrian
Copy link

bcbrian commented Jun 2, 2016

This fix would be amazing! 👍

When using practicalmeteor:mocha-console-runner a long with spacejam the following error is thrown. There is no route for the path: /local because it is going to the route http://localhost:3100/local. This cause my console unit tests to fail.

I currently have this file loaded at the beginning of all my *.tests.jsx and client side *.tests.js.

import { FlowRouter } from 'meteor/kadira:flow-router';

FlowRouter.route('/', {
  name: 'hack route for tests',
  action() {},
});
FlowRouter.route('/local', {
  name: 'hack route for tests',
  action() {},
});

This hackish solution works, but a fix in the code like suggested would do wonders.

@arunoda How do you handle this issue currently?

@rafa-munoz
Copy link

👍 It's really annoying that message

console.error("There is no route for the path:", context.path);
if(!Meteor.isTest && !Meteor.isPackageTest) {
console.error("There is no route for the path:", context.path);
}
Copy link

@serut serut Jul 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a else branchement with a log ?

} else {
    console.log("There is no route for the path:", context.path);
    if (window.console && window.console.trace) {
        console.trace();
    }
}

Otherwise 👍

@serut
Copy link

serut commented Jul 29, 2016

This pull request is working so great ! When you are running an app in the test mode, the library doesn't have any route and it just crashes. The test if(!Meteor.isTest && !Meteor.isPackageTest) is perfect for ignoring these errors.

console.trace is not implemented on phantomjs so my previous note is useless.

https://github.com/serut/todos use that fix and it works great.

It's related to A LOT of issues, so please do something to fix it:
practicalmeteor/spacejam#65
meteor/todos#129
#668
serut/meteor-coverage-app-exemple#1
https://forums.meteor.com/t/spacejam-phantomjs-no-route-for-the-path-local-flowrouter/25233
https://forums.meteor.com/t/no-route-for-path-error-running-unit-tests/21560/4
It's time to react... !

@s-devaney
Copy link

+1 please merge

@diegonc
Copy link

diegonc commented Sep 6, 2016

This is just hiding an annoying message (console.error doesn't throw thus the control flow is not interrupted) and not fixing the underlying problem.

Ok, I'm not sure about console runners maybe there it is a problem like the message being printed to stderr and confusing the runner. But the web driver has other problems too.

When running meteor test with the practicalmeteor:mocha driver, the page displayed in the browser that summarises the results has links to browse them (go into one section, go up, etc) which don't work
at all when flow-router is installed even if this patch is applied. @bcbrian hack didn't work either because flow-router still intercepts the location change.

What seems to work is

  • Intercepting clicks before flow-router gets them

    if (Meteor.isClient) {
        FlowRouter.wait();
        Meteor.startup(function () {
            document.addEventListener("click" , function (e) {
                // ensure link
                // use shadow dom when available
                var el = e.path ? e.path[0] : e.target;
                while (el && 'A' !== el.nodeName) el = el.parentNode;
                if (!el || 'A' !== el.nodeName) return;
    
                /* don't allow flow router to handle the event */
                e.stopPropagation();
                e.stopImmediatePropagation();
            });
            FlowRouter.initialize();
        });
    }
    
  • Or alternatively, leaving flow-router wait forever 😄

    if (Meteor.isClient) {
        FlowRouter.wait();
    }
    
  • Adding rel="external" to links generated by practical:mocha (flow-router ignores them, do other routers do the same though?)

@HugoHeneault
Copy link

Could this be merged soon? 👍

@aadamsx
Copy link

aadamsx commented Sep 28, 2016

Would you mind submitting this PR to FineRouter too?

danwild added a commit to danwild/fine-router that referenced this pull request Aug 8, 2017
Based on the PR that is still not merged into flowrouter
kadirahq/flow-router#615
@SimonSimCity
Copy link

Found this while still struggling. Here's a work-around:

Create a file that is loaded by the test-suite (by e.g. calling it routing.test.js) containing the following lines:

import { FlowRouter } from 'meteor/kadira:flow-router';

// Needed to initialize the routing when testing.
FlowRouter.notFound = {
  action() {
  },
};

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.

9 participants