-
Notifications
You must be signed in to change notification settings - Fork 119
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
solution #92
base: main
Are you sure you want to change the base?
solution #92
Conversation
944d827
to
dfe06a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like your GitHub account is not registered with JobSimulator.Dev. Please sign up at https://www.jobsimulator.dev to have your PR be tested automatically. Once you've signed up, close and open your PR to start the tests. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to see a new workflow file added for automated testing! This will definitely help to ensure code quality and streamline the testing process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's recommended to use an LTS version of Node.js for better stability and long-term support. Consider changing the node-version to 16 instead of 18.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice usage of npm ci
for a deterministic dependency installation! This ensures that the installed dependencies match the lockfile exactly, making the build process more consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you've added a package-lock.json
file in the .github/workflows
directory. Typically, package-lock.json
should be at the root level of the project, alongside the package.json
file. Please move the package-lock.json
file to the correct location.
.github/workflows/package.json
Outdated
{ | ||
"type": "module", | ||
"devDependencies": { | ||
"jsdom": "^20.0.2", | ||
"uvu": "^0.5.6", | ||
"puppeteer": "^19.7.2" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job adding the new package.json file with the required devDependencies and an empty scripts object. This sets up the foundation for future development.
.github/workflows/test.js
Outdated
import { setTimeout } from "timers/promises"; | ||
import { test } from "uvu"; | ||
import * as assert from "uvu/assert"; | ||
import * as backend from "./setup/backend.js"; | ||
import * as Puppeteer from "./setup/puppeteer.js"; | ||
import * as webpack from "./setup/webpack.js"; | ||
|
||
test.before(async (context) => { | ||
await webpack.build(); | ||
await webpack.serve(); | ||
await backend.start(); | ||
await Puppeteer.setup(context); | ||
}); | ||
|
||
test.after(async (context) => { | ||
await Puppeteer.reset(context); | ||
backend.stop(); | ||
webpack.stopServing(); | ||
}); | ||
|
||
test("Solved Issue #1: Company Names are Present", async (context) => { | ||
await Puppeteer.homepage(context); | ||
await setTimeout(250); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(2) > td:nth-child(1)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "Fusion LLC"); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(3) > td:nth-child(1)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "Techopolis Ltd."); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(4) > td:nth-child(1)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "Code learning LLC"); | ||
}); | ||
|
||
test("Solved Issue #2: display dates in 24-hour time format", async (context) => { | ||
await Puppeteer.homepage(context); | ||
await setTimeout(250); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(2) > td:nth-child(3)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "23:41"); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(3) > td:nth-child(3)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "03:45"); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(4) > td:nth-child(3)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "08:45"); | ||
}); | ||
|
||
test("Solved Issue #3: display revenue numbers in a human readable format", async (context) => { | ||
await Puppeteer.homepage(context); | ||
await setTimeout(250); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(2) > td:nth-child(4)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "17 000 000"); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(3) > td:nth-child(4)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "7 375 294"); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(4) > td:nth-child(4)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "100 000"); | ||
}); | ||
|
||
test("Solved Issue #4: make table look prettier", async (context) => { | ||
await Puppeteer.homepage(context); | ||
await setTimeout(250); | ||
|
||
var headerColor = await context.page.evaluate(() => { | ||
let header = document.querySelector("body > table > tbody > tr:nth-child(1)"); | ||
return window.getComputedStyle(header).getPropertyValue("background-color"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on solving the issues and adding tests for them! Your code looks clean and well-organized. Keep up the good work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice update to the .gitignore file to ensure all 'node_modules' directories are ignored, not just the top-level one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job setting up the GitHub Actions workflow! It's well-structured and easy to understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like the package-lock.json file has been added inside the .github/workflows directory. Please move it to the root of the project, as it should be placed alongside the package.json file.
.github/workflows/package.json
Outdated
{ | ||
"type": "module", | ||
"devDependencies": { | ||
"jsdom": "^20.0.2", | ||
"uvu": "^0.5.6", | ||
"puppeteer": "^19.7.2" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition of the package.json
file in the .github/workflows
directory! This will help manage the dependencies and scripts for your GitHub Actions workflows. Just a small suggestion: consider adding a name
and version
field to provide more information about this specific package configuration.
.github/workflows/test.js
Outdated
import { setTimeout } from "timers/promises"; | ||
import { test } from "uvu"; | ||
import * as assert from "uvu/assert"; | ||
import * as backend from "./setup/backend.js"; | ||
import * as Puppeteer from "./setup/puppeteer.js"; | ||
import * as webpack from "./setup/webpack.js"; | ||
|
||
test.before(async (context) => { | ||
await webpack.build(); | ||
await webpack.serve(); | ||
await backend.start(); | ||
await Puppeteer.setup(context); | ||
}); | ||
|
||
test.after(async (context) => { | ||
await Puppeteer.reset(context); | ||
backend.stop(); | ||
webpack.stopServing(); | ||
}); | ||
|
||
test("Solved Issue #1: Company Names are Present", async (context) => { | ||
await Puppeteer.homepage(context); | ||
await setTimeout(250); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(2) > td:nth-child(1)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "Fusion LLC"); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(3) > td:nth-child(1)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "Techopolis Ltd."); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(4) > td:nth-child(1)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "Code learning LLC"); | ||
}); | ||
|
||
test("Solved Issue #2: display dates in 24-hour time format", async (context) => { | ||
await Puppeteer.homepage(context); | ||
await setTimeout(250); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(2) > td:nth-child(3)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "23:41"); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(3) > td:nth-child(3)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "03:45"); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(4) > td:nth-child(3)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "08:45"); | ||
}); | ||
|
||
test("Solved Issue #3: display revenue numbers in a human readable format", async (context) => { | ||
await Puppeteer.homepage(context); | ||
await setTimeout(250); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(2) > td:nth-child(4)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "17 000 000"); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(3) > td:nth-child(4)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "7 375 294"); | ||
|
||
var text = await context.page.evaluate(() => { | ||
return document.querySelector("body > table > tbody > tr:nth-child(4) > td:nth-child(4)").textContent; | ||
}); | ||
assert.type(text, "string"); | ||
assert.is(text, "100 000"); | ||
}); | ||
|
||
test("Solved Issue #4: make table look prettier", async (context) => { | ||
await Puppeteer.homepage(context); | ||
await setTimeout(250); | ||
|
||
var headerColor = await context.page.evaluate(() => { | ||
let header = document.querySelector("body > table > tbody > tr:nth-child(1)"); | ||
return window.getComputedStyle(header).getPropertyValue("background-color"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on addressing all the issues mentioned in the PR! The tests are well-structured and cover the necessary cases. Keep up the good work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job updating the .gitignore to ignore node_modules in all subdirectories! This will help keep the repository clean from unnecessary files.
No description provided.