Skip to content

tests: add tests for project #45

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

Conversation

prabalsingh24
Copy link
Contributor

  1. Added tests for ProjectScenario
  2. Added tests for NodeJSProject

@prabalsingh24
Copy link
Contributor Author

prabalsingh24 commented May 26, 2020

The tests were talking a lot of time. See. 62000ms
But there were times when this was passing in 27000ms. Is it because of my laptop? You test it out too

Screenshot from 2020-05-26 13-33-49

I changed the timeout to 100000ms.

Copy link
Contributor Author

@prabalsingh24 prabalsingh24 left a comment

Choose a reason for hiding this comment

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

I've added the tests. Have a look at these comments :)

Comment on lines 31 to 46
let result
try {
// Setup RUNBOX
await scenario.setup(currentJobDir, job)

// Run worker
await scenario.run(currentJobDir, job)
// Run worker
await scenario.run(currentJobDir, job)

// Get result
const result = await scenario.result(currentJobDir, job)
// Get result
result = await scenario.result(currentJobDir, job)
}
catch(err) {
result = {
id: job.id,
stderr: err
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think of this try catch block. Earlier if there was any error in the code, the code would break and server would stop. So I think try catch is required

There is one problem with returning only id and stderr. The judge-api-queue-logic will fail and it will always think that this is run_result event

Copy link
Contributor

Choose a reason for hiding this comment

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

We do have an error queue mechanism so won't be needing try catch here

Comment on lines 53 to 67
it('should not break when url is invalid', async () => {
const projectResult = await execute(new ProjectJob({
id: 25,
source: 'https://www.invalidurl.com',
problem: 'https://www.invalidurl.com',
submissionDirs: 'src/*',
lang: 'nodejs',
timelimit: 20,
scenario: 'project'
}))
expect(projectResult).to.have.keys(
'id',
'stderr',
)
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the case when the code was breaking. Because this url will pass the validator of judge-api

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically we should expect execute function to throw an error

Comment on lines 91 to 106
it('should not break if lang is incorrect', async () => {
const projectResult = await execute(new ProjectJob({
id: 25,
source: 'https://minio.cb.lk/public/problem.zip',
problem: 'https://minio.cb.lk/public/solution.zip',
submissionDirs: 'src/*',
lang: 'abcd',
timelimit: 20,
scenario: 'project'
}))

expect(projectResult).to.have.keys(
'id',
'stderr',
)
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This won't be happening. Lang can't be incorrect. Because it's been checked in the validator. But it's better to be safe :)

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't have a clear way of reporting wrong lang error in taskmaster. Let's skip as of now

@prabalsingh24 prabalsingh24 force-pushed the project-worker-nodejs-tests branch from c0ede84 to 5f364bb Compare May 27, 2020 06:24
@prabalsingh24
Copy link
Contributor Author

prabalsingh24 commented May 27, 2020

EDIT: All the Travis CI tests failed because of sercurity certificate issue

the url's security certificate expired? or this issue related to my internet

Screenshot from 2020-05-27 11-56-51

@prabalsingh24
Copy link
Contributor Author

@jatinkatyal13 .

@jatinkatyal13 jatinkatyal13 merged commit 0bac7b4 into coding-blocks:project-worker Jun 9, 2020
@boss-contributions-bot
Copy link

Congratualtions @prabalsingh24, your pull request is merged! 🎉

Thanks for your contributions and participating in BOSS 2020. 🙌

You can claim your bounty points here. 💰

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants