Fix test of fileupload

[f-higashi]

I added FileDetector setting to test file upload in saucelabs.
FileDetector locates selenium-webdriver/remote/index.js.
But the file path is different between npm2 and npm3.
https://docs.npmjs.com/how-npm-works/npm3-nondet

npm in travis is v2.
If we want to use v3 in travis, we need to install npm3.

.travis.yaml

before_install: if [[ `npm -v` != 3* ]]; then npm i -g npm@3; fi

Which version is better?
Do you have a good idea to support both versions?

@bryk @floreks @maciaszczykm

---

This change is

[codecov-io]

Current coverage is 86.54%

Merging #515 into master will not affect coverage as of a43bf51

@@            master    #515   diff @@
======================================
  Files           97      97       
  Stmts          847     847       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit            733     733       
  Partial          0       0       
  Missed         114     114       

Review entire Coverage Diff as of a43bf51

Powered by Codecov. Updated on successful CI builds.

[maciaszczykm]

Could you please rebase commits into one? If you want to split your contribution then best idea is to do it in multiple pull requests.

[f-higashi]

It's OK.
But I will do after a decision that we use npm2 or npm3.

If we decide to use npm v2, I remove 1acbe14 easily.

[floreks]

NPM 3 is fine for me. As long as we are using npm install we should end up with the same dep. tree.

[floreks]

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

src/test/integration/deploy/deployfromfile_po.js, line 42 [r1] (raw file):
Although I don't like usage of document here I don't see any other possible solution for that. @bryk do you have any other idea?

---

src/test/integration/deploy/deployfromfile_po.js, line 45 [r1] (raw file):
Do we actually need to change height and width from 0 to 1px? Isn't visible sufficient?

---

src/test/integration/stories/deploy_from_invalid_file_test.js, line 19 [r1] (raw file):
Remove newlines, try to sort it in alphabetic order and separate external imports from test file imports. It's easier to read this way.

You can just use:

import path from 'path';
import remote from 'selenium-webdriver/remote';

import DeployFromFilePageObject from '../deploy/deployfromfile_po';

---

src/test/integration/stories/deploy_from_valid_file_test.js, line 22 [r1] (raw file):
Same here.

---

Comments from the review on Reviewable.io

[floreks]

This is really nice work. Just few small comments.

[f-higashi]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

src/test/integration/deploy/deployfromfile_po.js, line 42 [r1] (raw file):
How about the following method in deployfromfile_po.js.
And this.filePciker changes private variable.

In this case I can remove usage.

  /**
   * Sets file into filePicker
   * @param {string} filePath
   */
  setFile(filePath) {
    /**
     * Firefox does not allow sendKeys to invisible input[type=file] element.
     */
    let makeInputVisible = function() {
      /* global document */
      let filePickerDomElement = document.getElementsByClassName('kd-upload-file-picker')[0];
      filePickerDomElement.style.visibility = 'visible';
      filePickerDomElement.style.height = '1px';
      filePickerDomElement.style.width = '1px';
    };
    browser.driver.executeScript(makeInputVisible);
    this.filePicker_.sendKeys(filePath);
  }

In test code

    // when
    deployFromFilePage.setFile(absolutePath);
    deployFromFilePage.deployButton.click();

---

src/test/integration/deploy/deployfromfile_po.js, line 45 [r1] (raw file):
I referred bellow page;
http://stackoverflow.com/questions/6101461/how-to-force-selenium-webdriver-to-click-on-element-which-is-not-currently-visib
It's about button.
In fact our tests did not work when I set only visible.

---

Comments from the review on Reviewable.io

[floreks]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

src/test/integration/deploy/deployfromfile_po.js, line 42 [r1] (raw file):
I don't think this is a good idea. This part of code

browser.driver.executeScript(makeInputVisible);
this.filePicker_.sendKeys(filePath);

belongs to test and should not be executed in PO file. PO should be only for retrieving elements on page and logic related to that. It should be clear and visible in test that we are executing some script on page and sending input to field.

What do you think?

---

src/test/integration/deploy/deployfromfile_po.js, line 45 [r1] (raw file):
Ok. Thanks for explanation.

---

Comments from the review on Reviewable.io

[f-higashi]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

src/test/integration/deploy/deployfromfile_po.js, line 42 [r1] (raw file):
I agree that PO should be only for retrieving elements on page ~.

On the web page we provides fileName input and ... button instead of filePicker.
fileName.sendsKey does not work.
I don't think it is bad that we provide other function setFile instead of filePicker.
I could say setFile is one of Services in this page such as typeUsername of the following example.

https://github.com/SeleniumHQ/selenium/wiki/PageObjects#summary

But setFile changes PO status.
It is better to reset css in this fucntion.

ex)

browser.driver.executeScript(makeInputVisible);
this.filePicker_.sendKeys(filePath);
browser.driver.executeScript(restoreVisibility);

---

Comments from the review on Reviewable.io

[floreks]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

src/test/integration/deploy/deployfromfile_po.js, line 42 [r1] (raw file):
What about making 2 functions in PO

setFileName(path) that will basically do this.filePickerElement.sendKeys() and
makeInputVisible() that will execute browser.driver.executeScript(() => {}) with inline function.

Then we can call in test:

po.makeInputvisible();
po.setFileName(path);

I think restore vibisility is not needed. Style will be restored anyway when page will be reloaded or state changes.

---

Comments from the review on Reviewable.io

[f-higashi]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

src/test/integration/deploy/deployfromfile_po.js, line 42 [r1] (raw file):
It looks good.
I'll fix it.
Thanks.

---

Comments from the review on Reviewable.io

[f-higashi]

PTAL

[floreks]

Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

src/test/integration/deploy/deployfromfile_po.js, line 37 [r2] (raw file):
I think it should be Make filePicker input field visible

---

src/test/integration/deploy/deployfromfile_po.js, line 51 [r2] (raw file):
Actually it only Sets filepath on the filePicker input field

---

src/test/integration/stories/deploy_from_invalid_file_test.js, line 21 [r2] (raw file):
This comment is no longer needed in both test files.

---

Comments from the review on Reviewable.io

[floreks]

Last style comments and we can merge

---

Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

Comments from the review on Reviewable.io

[f-higashi]

Thanks for your comment.
I fixed them.

PTAL

[floreks]

Reviewed 3 of 3 files at r3.
Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

Comments from the review on Reviewable.io

[floreks]

---

Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

Comments from the review on Reviewable.io

[floreks]

@bryk could you shortly check that?

[bryk]

One comment for simplification.

---

Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

.travis.yml, line 39 [r3] (raw file):
Instead of this, you can specify node_js version to 5.x.y which is bundled with npm 3. Do this, it'll avoid this little hack :)

See https://nodejs.org/en/blog/release/v5.0.0/

---

Comments from the review on Reviewable.io

[f-higashi]

Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

.travis.yml, line 39 [r3] (raw file):
Thanks for your comment.
I'll use 5.1.1 which is latest in travis.

https://docs.travis-ci.com/user/languages/javascript-with-nodejs

---

Comments from the review on Reviewable.io

[bryk]

---

Reviewed 2 of 2 files at r4.
Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks broke.

---

Comments from the review on Reviewable.io

[bryk]

Tests fail. Can you fix them?

[f-higashi]

Yes.
I check it on the way.

In my local pc tests success. but in travis test fail because test fail to delete created RC.

[bryk]

Can you fix the test?

[bryk]

And which test does not delete created RCs?

[f-higashi]

I cannot find the solution now.

deploy_from_valid_file_test create container and fail to delete the container in after_all.
Next test Zero state view should do something fail because the container is existing.

https://travis-ci.org/kubernetes/dashboard/builds/116599998

AfterAll Failed: No element found using locator: By.xpath("//md-dialog[@aria-label='Delete Replication Controller']//md-dialog-actions//span[text() = 'Delete']/..")

I tried to test in same version; node, npm, browser and dependencies library in local PC.
But tests succeeded in local PC.

And I reverted source which succeeded 3 days ago but tests has same error in travis.
Of course tests succeed in local PC.

Do you have any idea?

[floreks]

I believe this is related to #546. There had to be some minor update of some dependency in our project. Since we use ~1.0.0 versioning in bower/package json then it accepts all newest versions up to 1.0.x. Travis always downloads everything.

This xpath is now not correct. Anyway as discussed in #508 we should not use xpath and refactor the code that uses it.

There is no <span> anymore.

[f-higashi]

I see.
I deleted node_module but not bower_components..

I reproduced error in local PC.

Thanks for your help : )

[bryk]

Thanks for fixing this issue! Will merge when travis gets green.

---

Reviewed 2 of 2 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io