Skip to content
This repository has been archived by the owner on Jun 5, 2024. It is now read-only.

Feature/karma #113

Merged
merged 8 commits into from
Nov 15, 2018
Merged

Feature/karma #113

merged 8 commits into from
Nov 15, 2018

Conversation

DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented Nov 13, 2018

Description

Move testing over to karma and test in browser.

ToDo - maybe followup PRs

  • move remaining tests
  • enable files tests
  • add coverage
  • move away from browserification

Copy link

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

See comments

// test results reporter to use
// possible values: 'dots', 'progress'
// available reporters: https://npmjs.org/browse/keyword/karma-reporter
reporters: ['dots'],

Choose a reason for hiding this comment

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

later on you can enable code coverage output here


describe("Currently testing apps management,", function() {
// CURRENT TIME
var timeRightNow = new Date().getTime();

Choose a reason for hiding this comment

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

if you import the "sinon" JS library, you can also use fake timers to reduce randomness of time-related tests

// there are cors issues with this api
describe("App attributes testing,", function() {
beforeEach(function(done) {
var key = ['attr1', 'attr+plus space', '属性1'];

Choose a reason for hiding this comment

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

weird indent

/* globals ownCloud, __karma__ */

// CURRENT TIME
var timeRightNow = new Date().getTime();

Choose a reason for hiding this comment

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

I suggest moving all those vars into the first describe() block. There's a risk that these will conflict with the values from other files if loaded in sequence.

Copy link
Member Author

Choose a reason for hiding this comment

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

in this case I need to duplicate all these vars into two describe blocks - right?

Choose a reason for hiding this comment

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

oh, I didn't noticed that. Then wrap the whole file in another describe() block then that describes the whole file...

@DeepDiver1975
Copy link
Member Author

remaining todos will come in followup prs ->merge

@DeepDiver1975 DeepDiver1975 merged commit 1b8bf06 into master Nov 15, 2018
@DeepDiver1975 DeepDiver1975 deleted the feature/karma branch November 15, 2018 22:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants