Issue Tracker - Problem with test suite (multiple filters)

Upon getting to the “Multiple Filters” test in the test suite, my code either:

  1. Doesn’t mark it as being passed despite my implementation working (and even when I try to force it to mark as completed by only having a “done();”)

  2. “Skips” a test, changing the name of a test to the next one while not passing, e.g:

 ✓ One filter (110ms)
 1) Multiple filters (test for multiple fields you know will be in the db for a return)
//here are actually some console logs I removed but will include later
DELETE /api/issues/{project} => text
 ✓ No _id
//This test should be "Valid _id", not a duplicate "No _id"
 ✓ No _id (110ms)
  1. Passes the test like it’s supposed to, but then throws an error and causes the subsequent tests to do either #1 or #2, e.g:
      ✓ Multiple filters (test for multiple fields you know will be in the db for a return) (39ms)
    DELETE /api/issues/{project} => text
//A note here: I haven't done any of the Delete yet, so they are empty besides "done();"
      1) No _id

/app/assertion-analyser.js:103
  var body = body.match(/(?:browser\s*\.\s*)?assert\s*\.\s*\w*\([\s\S]*\)/)[0];
                                                                           ^
TypeError: Cannot read property '0' of null
at assertionAnalyser (/app/assertion-analyser.js:103:76)
at Runner.<anonymous> (/app/test-runner.js:69:23)
    at Runner.emit (events.js:194:15)
    at Runner.uncaught (/rbd/pnpm-volume/0efca928-96b8-47b7-9e5b-9cf6a28e8d4d/node_modules/.registry.npmjs.org/mocha/3.5.3/node_modules/mocha/lib/runner.js:720:10)
    at process.uncaught (/rbd/pnpm-volume/0efca928-96b8-47b7-9e5b-9cf6a28e8d4d/node_modules/.registry.npmjs.org/mocha/3.5.3/node_modules/mocha/lib/runner.js:804:10)
    at process.emit (events.js:189:13)
    at process._fatalException (internal/bootstrap/node.js:496:27)




My code for the “One Filter” test is as follows:

      test('One filter', function(done) {
        chai.request(server)
          .get('/api/issues/test')
          .query({issue_title:'Title 2'})
          .end(function(err, res){
            console.log(res.body[0]);
            console.log(res.body[1]);
            assert.equal(res.status, 200);
            assert.isArray(res.body);
            assert.equal(res.body[0]['issue_title'], 'Title 2');
            assert.equal(res.body[1]['issue_title'], 'Title 2');
            done();
          });
      });

With the console.logs consisting of:

    GET /api/issues/{project} => Array of objects with issue data
      ✓ No filter (1161ms)
{ _id: '5ead39038056522c2c9fed32',
  issue_title: 'Title 2',
  issue_text: 'text 2',
  created_by: 'Owner',
  assigned_to: '',
  status_text: '',
  open: true,
  created_on: '2020-05-02T09:10:27.929Z',
  updated_on: '2020-05-02T09:10:27.929Z' }
{ _id: '5ead39b463c6072d6b428a58',
  issue_title: 'Title 2',
  issue_text: 'text 2',
  created_by: 'Owner',
  assigned_to: '',
  status_text: '',
  open: true,
  created_on: '2020-05-02T09:13:24.150Z',
  updated_on: '2020-05-02T09:13:24.150Z' }
      ✓ One filter (110ms)

And my code for the “Multiple Filters” test is:

test('Multiple filters (test for multiple fields you know will be in the db for a return)', function(done) {
        chai.request(server)
          .get('/api/issues/test')
          .query({_id: "5ead39038056522c2c9fed32", issue_title: 'Title 2'})
          .end(function(err, res){
            console.log(res.body);
            console.log(res.body[0]['_id']);
            console.log(res.body[0]['issue_title']);
            assert.equal(res.status, 200);
            assert.isArray(res.body);
            assert.equal(res.body[0]["_id"], '5ead39038056522c2c9fed32');
            assert.equal(res.body[0]['issue_title'], 'Title 2');
            done();
          });

Which logs to the console:

[ { _id: '5ead39038056522c2c9fed32',
    issue_title: 'Title 2',
    issue_text: 'text 2',
    created_by: 'Owner',
    assigned_to: '',
    status_text: '',
    open: true,
    created_on: '2020-05-02T09:10:27.929Z',
    updated_on: '2020-05-02T09:10:27.929Z' } ]
5ead39038056522c2c9fed32
Title 2
      ✓ Multiple filters (test for multiple fields you know will be in the db for a return) (39ms)

Again, I’d like to mention that sometimes it will pass, and sometimes it will not, even if all I have in the “end()” function is a done();

My “delete” test suite is only as follows:

suite('DELETE /api/issues/{project} => text', function() {
      
      test('No _id', function(done) {
        chai.request(server)
          .get('/api/issues/test')
          .end(function(err, res){
            done();
          });
      });
      
      test('Valid _id', function(done) {
        chai.request(server)
          .get('/api/issues/test')
          .end(function(err, res){
            done();
          });
      });
      
    });




All of these different logs have been from only messing with the test suite itself; my code for the actual GET request has remained the same. I can provide it if it’s necessary to figure out what’s going on, but it’s my understanding that my code is working as intended as far as that goes, and that I’m doing something wrong in/with the code for the test suite. Any help would be greatly appreciated.

Hello!

Can you provide your github repository link (or glitch project)?

Sure thing- https://glitch.com/edit/#!/jasper-shining-loganberry

There are a few issues…

First, when you connect to the database, the URL should already have the database name set, like this:

mongodb://some_ip:27017/the_database_name?otherParams=blah

But when you write this:

var db = client.db(project);

You’re instructing the mongo driver to change to the database specified by project variable, which is not wrong, but it will lead to unexpected results.

Besides that, you should use the err when they’re filled. On your tests, you’re doing something like this:

test('description', function(done) {
  chai.request(server)
        .post('/api/issues/test')
  .end(function(err, res) {
    // Here are your tests
    // Finally the call to done
    done();
  });

The problem with that is that if the err variable has something, you’re completely ignoring it (apart from the fact that your tests will run on faulty data), which leads to problems in the future. Rewrite it like this:

test('description', function(done) {
  chai.request(server)
        .post('/api/issues/test')
  .end(function(err, res) {
    // The first thing when testing API calls:
    if (err) {
      // If you don't return, the function continues its execution
      // console.error(err); // It may be useful if you didn't log the error previously
      return done(err);
    }
    // Here are your tests
    // Finally the call to done
    done();
  });

Hope it helps :slight_smile:,

Happy coding!

You’re instructing the mongo driver to change to the database specified by project variable, which is not wrong , but it will lead to unexpected results.

What exactly do you mean by unexpected results? My take was that I had to have it effectively be able to access two separate databases based on the projects parameter- one for my tests (from the given “/api/issues/test”), and one for the actual database used at the end ("/api/issues/apitest"). I figured this was the point of them providing the “var project = req.params.project” at the start of each request code block, and so far I haven’t had any issues or results that I would think were unexpected as far as where things have been going in my databases, so I’m curious as to why this is wrong. Is it the right idea, wrong implementation? What would be a better way to utilize the provided code in this regard?

As far as the rest goes, I’ll look in to it. I haven’t been utilizing the ‘err’ variable, you’re correct. I guess I glossed over it because none of the provided test cases in any of the suites touched on it. Hopefully it ends up shedding some light on to all of this. Thanks for your time looking in to this.

EDIT: I’d also like to ask- what is the reason that it wouldn’t pass the test if the only code in it is “done()” (like they way I set up my DELETE tests)? Shouldn’t it just mark the test as completed whether there are errors or not?

EDIT 2: I’ve rewritten the code for one of my test to be as follows:

test('Multiple filters (test for multiple fields you know will be in the db for a return)', function(done) {
        chai.request(server)
          .get('/api/issues/test')
          .query({_id: "5ead39038056522c2c9fed32", issue_title: 'Title 2'})
          .end(function(err, res){
            if(err){
              console.log(err);
              return done(err);
            }
            console.log(res.body);
            console.log(res.body[0]['_id']);
            console.log(res.body[0]['issue_title']);
            console.log(res.status);
            assert.equal(res.status, 200);
            //assert.isArray(res.body);
            //assert.equal(res.body[0]["_id"], '5ead39038056522c2c9fed32');
            //assert.equal(res.body[0]['issue_title'], 'Title 2');
            done();
          });
      });

And it logs to the console:

      ✓ One filter (70ms)
      1) Multiple filters (test for multiple fields you know will be in the db for a return)
    DELETE /api/issues/{project} => text
[ { _id: '5ead39038056522c2c9fed32',
    issue_title: 'Title 2',
    issue_text: 'text 2',
    created_by: 'Owner',
    assigned_to: '',
    status_text: '',
    open: true,
    created_on: '2020-05-02T09:10:27.929Z',
    updated_on: '2020-05-02T09:10:27.929Z' } ]
5ead39038056522c2c9fed32
Title 2
200
      ✓ No _id
      ✓ No _id (73ms)

Which to me seems that it skips the if(err) block and goes on to the rest of it, meaning that there is no error being caught… it should then just go on and pass (only asserting that the status is 200). To save time, I’ve also added the same "if(err) block to the One Filter test, and that doesn’t catch anything as well.

Oh! I see… In that case the implementation is wrong. If you want to use a test database and a different production database, then you should be using the .env file (or environment variables) to specify a different one.

The URIs you mention are expected to be different projects in the context of an issue, not in the context of your issue tracker. Look at github.com/freeCodeCamp/freeCodeCamp/issues, for instance, to see what the expected result should be (keep in mind that you’re not expected to follow the URL format exactly, like userName/projectName/issues, it’s just an idea).

Let me know I was clear or you need more clarification on this :stuck_out_tongue:.

To be honest, I don’t know. I didn’t test it :stuck_out_tongue:. This should not happen, but it may happen if we forgot to remove/comment or otherwise clean from the block of code; for instance, a call to an endpoint, a semicolon (yep, in some languages, this may create errors xD), etc. There are times that errors that fail to be caught by the test runner (mocha, for instance) may propagate to the next test… The mysteries of the code (I’m kidding, there’s always an explanation, but it may require a deep dive into the code).

That’s fine :slight_smile:. It will help you debug in case something happens when you edit files (or if the database goes offline, for instance).

Sorry for the late response- my power just went out for a bit.

The URIs you mention are expected to be different projects in the context of an issue, not in the context of your issue tracker .

I see… I think I understand what you’re saying. Is it that my overall database should be determined by my URI (in this case, the given “name” to use was DB from the process.env.DB), and that I would just switch my environment variable when I was done working on my test database and ready to go live (pointing it at ?apitest instead of ?test)? And the “var project = req.params.project” parameter that was given would instead be to start a new collection, and that collection would hold the issues for that given “project”? In my case, instead of making a random collection name like I do in "db.collection(‘issues’) I would do something like db.collection(project)? If so, is this done more because it’s “proper”, or for better security?

I’ve added the if(err) block to my DELETE tests, and now it seems that the other tests are passing (inconsistently), but it still runs in to this error:

    DELETE /api/issues/{project} => text
      1) No _id
/app/assertion-analyser.js:103
  var body = body.match(/(?:browser\s*\.\s*)?assert\s*\.\s*\w*\([\s\S]*\)/)[0];
                                                                           ^
TypeError: Cannot read property '0' of null
at assertionAnalyser (/app/assertion-analyser.js:103:76)
at Runner.<anonymous> (/app/test-runner.js:69:23)
    at Runner.emit (events.js:194:15)
    at Runner.uncaught (/rbd/pnpm-volume/0efca928-96b8-47b7-9e5b-9cf6a28e8d4d/node_modules/.registry.npmjs.org/mocha/3.5.3/node_modules/mocha/lib/runner.js:720:10)
    at process.uncaught (/rbd/pnpm-volume/0efca928-96b8-47b7-9e5b-9cf6a28e8d4d/node_modules/.registry.npmjs.org/mocha/3.5.3/node_modules/mocha/lib/runner.js:804:10)
    at process.emit (events.js:189:13)
    at process._fatalException (internal/bootstrap/node.js:496:27)

once it gets to my first DELETE test. That test shouldn’t be doing anything as I haven’t even started it yet, and I had never run in to this error when the rest of my request code blocks were blank like this one is:

.delete(function (req, res){
      var project = req.params.project;
      
    });

Even changing my DELETE tests to the base

test('No _id', function(done) {
        
      });

doesn’t help, and none of the error blocks in the previous tests are catching anything. This error has shown up before, too, as I put in my initial post. Do you have any ideas why assertionAnalyser is throwing this error all of a sudden? It seems to only show the error once it hits the “No _id” test, making me think that the issue lies there, but I can’t think of a reason why it would only trigger with this test and not any of the other ones if they all started in the same blank state.

Alright, I fixed my problem and will try to explain it the best I can here in case anyone comes across this later and has a similar problem.

First, I mistakenly had a GET request instead of the proper DELETE requests in my DELETE test suite, which is obviously no good. Changing those didn’t help much as nothing was supposed to be happening yet anyways, but it would have caused problems later.

The “assertionAnalyser” error

/app/assertion-analyser.js:103
  var body = body.match(/(?:browser\s*\.\s*)?assert\s*\.\s*\w*\([\s\S]*\)/)[0];
                                                                           ^
TypeError: Cannot read property '0' of null
at assertionAnalyser (/app/assertion-analyser.js:103:76)
at Runner.<anonymous> (/app/test-runner.js:69:23)
    at Runner.emit (events.js:194:15)
    at Runner.uncaught (/rbd/pnpm-volume/0efca928-96b8-47b7-9e5b-9cf6a28e8d4d/node_modules/.registry.npmjs.org/mocha/3.5.3/node_modules/mocha/lib/runner.js:720:10)
    at process.uncaught (/rbd/pnpm-volume/0efca928-96b8-47b7-9e5b-9cf6a28e8d4d/node_modules/.registry.npmjs.org/mocha/3.5.3/node_modules/mocha/lib/runner.js:804:10)
    at process.emit (events.js:189:13)
    at process._fatalException (internal/bootstrap/node.js:496:27)

Seems to happen if you don’t have any assertions (assert.equal(…), etc.) in your test block. It seems to trigger an error even if the block just has:

chai.request(server)
          .delete('/api/issues/test')
          .end(function(err, res){
            if(err){
              console.log(err);
              return done(err);
            }
            done();
          })

Adding in any assertion fixes this, though I still don’t understand why it never triggered the rest of the time.



After fixing those, I was able to discover that the real error (which I only found by following Skaparates advice) was:

1) Functional Tests GET /api/issues/{project} => Array of objects with issue data Multiple filters (test for multiple fields you know will be in the db for a return):
     Uncaught Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client

my Multiple Filter test was trying to respond too many times. Changing my code from

if(Object.keys(req.query).length > 0){
            
    if(req.query._id){
        var newId = req.query._id
        req.query._id = ObjectId(newId)
    }
            
    var cursor = db.collection('issues').find(req.query)
    cursor.toArray(function(err, docs){
        if(err){
            console.log(err);
        } else {
            return res.send(docs);
        };
    });
}    
var cursor = db.collection('issues').find({})
cursor.toArray(function(err, docs){
    if(err){
        console.log(err);
    } else {
        return res.send(docs);
    };
});      

to

if(Object.keys(req.query).length > 0){
            
    if(req.query._id){
        var newId = req.query._id
        req.query._id = ObjectId(newId)
    }
            
    var cursor = db.collection('issues').find(req.query)
    cursor.toArray(function(err, docs){
        if(err){
            console.log(err);
        } else {
            return res.send(docs);
        };
   });
} else {
    var cursor = db.collection('issues').find({})
    cursor.toArray(function(err, docs){
        if(err){
            console.log(err);
        } else {
            return res.send(docs);
        };
    });
}

fixed it. I thought that the “return res.send(docs)” in the if block would stop the rest of the code from running, and it wasn’t. Wrapping the rest of the code in an else block stopped the problem.

1 Like

Not exactly. You can design your project however you want, but the FCC project is trying to push a more strict design.

In this project (and any REST project), the URL (what you refer to as apitest or test) are just parts of the end point; a route, nothing else. It’s not related to the database design. You could have a database for every collection, which would be fine (although we could argue that’s too much), but we usually have a single database with any number of collections.

You should define your collections as issues, projects and users (which are the some of the entities involved on an issue tracker) and those collections go to a single database called issue_tracker, for instance. When you’re testing, you could have a different database called issue_tracker_test (following the example), but maintaining the same collections, so your tests have a common ground.

The project is nothing more than the name of an object inside the collection (the projects collection, or however you name it). A project object inside the projects collection could look like this:

{
  "_id": "32abffad23",
  "name": "The project Name",
  "description": "This project does this and that",
  "owner": "David Underwood",
  "collaborators": ["Collab 1", "Collab 2"]
}

And the issues could look like this:

{
  "title": "Bug: No more stack",
  "project": "32abffad23" /* Reference to the project above */
}

This would be one way to create the objects. Of course, since mongo doesn’t enforce any kind of model, you can structure it however you want. You could remove the issues collection entirely and just push collection objects into a property of a project object.

In short, everything your endpoints (the routes you create on nodejs/express) receive is just data, and that data can be structured however you want inside a single or multiple databases (though you should use one, at least until you have more experience).

I won’t comment the rest of the problem since you’ve already solved it :slight_smile:, so congrats!

I think I’m a little more confused now, I’m sorry.

I understand how the URL is just a route to an endpoint, and that it’s not specific to any database design. The part that is confusing is the given parameter, the “project = req.params.project”, and why that is even given to us if it’s not meant to select a database (in my case) or a collection (they way I thought you meant it to be).

The only reason is that it doesn’t really make sense to me otherwise. The parameter is already chosen for you on the actual project page- every action there (such as submitting a new issue) is already sent to “/api/issues/apitest”, making “apitest” the given parameter for the route for any action performed on the “actual” database. The test suites provide you with “/api/issues/test” as a default route, though you can obviously change that to be whatever it is that you would like. But if this parameter was meant to be the name of an object inside a collection, wouldn’t that make it so that every object would be named “apitest” (or “test” or whatever) inside of that collection? And if I’m testing and should use a different database, how would I be able to have my code work on a different database without changing my .env variable?

EDIT: I guess my main issue is: Why even have a parameter in the route at all? What purpose does it serve in this case? It sounds like I could have done everything the same if it wasn’t there, so what is the point of including it and what way is it “intended” to be used?

The operation is meant to be performed on the object, not a specific database or collection.

A client (the browser, for instance) should be able to reach any project:

// Assume it's a get request
const project = req.params.project

// Find the object that matches the project name:
db.collection('projects').find({ project }, function(err, data) {
  if (err) {
    return res.json({ error: `We could not find the project ${project}`});
  }
  // Would return an object with { data: {...} } or null if not found
  return req.json({ data });
});

Yes, it’s not required there, but you need some way to reference a project. It could be on the request body instead of the URL:

req.body.project

You would use a different way to access the project name, that’s all.

There are many problems that you will have if you try to create a collection or database for every project.

For instance, a project name is user input, which can be dangerous. Now, mongodb has a limit of 64 characters per collection name, which means that if a user inputs a string with a length of more than 64 characters it would crash your app; if we take this into account, using collections per project is not a good idea (aside from the fact that collection names cannot contain certain characters).

If, instead, we add a property inside each object on a collection we can (with some safeguards) store a user value directly and we wouldn’t be polluting the database. Basically we are less restricted when using objects inside collections.

Hmmm, maybe if you take a look at mongoose may help you.

First of all, I appreciate you taking your time out to help me understand this more. It is definitely helping me out on learning this- I either missed something while taking these lessons, or it wasn’t explained in enough detail for me to grasp it.

I think I am beginning to understand what you’re saying, so I just want to clarify again to make sure.

The database I choose is irrelevant, but it would be chosen by my .env variable. This wouldn’t change (unless I chose to change it myself, but the code wouldn’t affect it). The collection I chose would work the same way; no matter the user input, I would have all my data going to a collection of my choosing- in this case, I might call it “issue-tracker” (db.collections(‘issue-tracker’)). The variable that I’m given, “project”, would be just to name an object. I could then have my Schema set up to be something like:

Schema:{
    project_name: SubSchema
}

and a SubSchema setup with the rest of the info, like:

SubSchema:{
    issue_name: {type:String},
    issue_text: {type:String},
    etc...
}

and then use the user input in the the route as the name in my Schema, and store the rest of the data as the SubSchema under that project name.

Does that sound closer to being correct?

Don’t worry, we all have different ways to understand things :slight_smile:.

Yes! You’re definitively closer now :partying_face:.

In time you’ll realize by yourself (with experience) that some designs simply don’t solve the problem you have in front.

The design that FCC tries to teach works most of the time, but it’s not a bullet proof.

1 Like

Great, then thank you for your time again. It helped me immensely with not only figuring out how to solve my own problem, but understanding a bit more on how I should be tackling these problems. Much appreciated!

1 Like