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

Add automated testing support #39

Merged
merged 7 commits into from May 26, 2019
Merged

Conversation

gervasiocaj
Copy link

@gervasiocaj gervasiocaj changed the title Add testing support Add automated testing support Mar 28, 2018
@nliautaud
Copy link
Owner

nliautaud commented Apr 11, 2018

Wooo, thanks 👍
I'm not sure if I'll be able to review that correctly as I never did such automated testing.

Could you give me some use case or quick demo ?

PS : there is extensive changes in #34 , it may be great to merge from there.

@AmineI
Copy link

AmineI commented Apr 10, 2019

Quick demo :

Firstly, install the required node modules, including mocha - a test framework, with npm install.

Then use npm test to launch the tests that are in test.js.

It basically launch a programmatically controlled chromium instance with your extension loaded, and does each test in there.

It uses mocha; here's a Getting Started Guide about it if you're not familiar with it.

Puppeteer is used to control the special Chromium instance.

@AmineI
Copy link

AmineI commented Apr 10, 2019

Two tests are included in tests.js, and it's up to us to create more

Using a user's list could cause issues honestly though. Plus @gervasiocaj isn't using staging.trakt.tv as suggested in trakt's API docs,

to avoid filling production with test data

  • The second test opens https://trakt.tv/movies/moana-2016 , translates its title in a few languages and check if the language code shown next to the title is the expected one.

Currently, the tests are failing : I believe it's because he uses pt-br for the test translations, which is not a ISO 639-1 code as required by the extension.

@AmineI
Copy link

AmineI commented Apr 10, 2019

Also, I'd move all these files in a subdirectory - they should not be included with the extension when shipped and should be properly separated.

@AmineI
Copy link

AmineI commented Apr 10, 2019

The 1st test is now passing, however the underlying issue isn't fixed, we'll have to look into it - As you noticed, pt-BR works but not pt-br. However, when submitted by an user in the options windows, the extension always change the entered string to lowercase- we bypassed since we were manually writing to the extension's storage.

I also went ahead and fixed the issue causing the second test to fail in #42.

@gervasiocaj
Copy link
Author

Will #34 be merged into master? If not, why?

@AmineI
Copy link

AmineI commented Apr 11, 2019

Will #34 be merged into master? If not, why?

I believe it'd be merged when deemed stable, so when the individual additions of 0.5 are tested - since @nliautaud marked them as "await testing". He's probably busy ATM though.

image

@AmineI
Copy link

AmineI commented Apr 11, 2019

You'd still have to edit the PR to set the target branch to nliautaud:0.5 so that only the correct diff/commits are shown :)

@gervasiocaj gervasiocaj changed the base branch from master to 0.5 April 11, 2019 15:53
@nliautaud
Copy link
Owner

Thanks both of you !
Busy indeed :)

@nliautaud nliautaud merged commit 3fe0b7d into nliautaud:0.5 May 26, 2019
@nliautaud nliautaud added this to the v0.5 milestone May 26, 2019
@gervasiocaj gervasiocaj deleted the testing branch April 13, 2022 07:04
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.

None yet

3 participants