search

blackboard / protractor-sync

Wrapper around Protractor, providing synchronous test writing and lots of helper functions.
MIT License
17 stars 5 forks source link

Fix Github security warning #83

Closed ftclausen closed 1 year ago

ftclausen commented 5 years ago

Also use package-lock.json to make builds more reproducible.

This is not quite ready to merge due to a failed test:

    Window size
      ✗ resizes the window
        - Expected 296 to equal 200.
            at /Users/fclausen/bbgit/protractor-sync/test/spec/protractor_sync_test.ts:805:30
            at /Users/fclausen/bbgit/protractor-sync/test/spec/protractor_sync_test.ts:46:7
            at fiberContents (/Users/fclausen/bbgit/protractor-sync/node_modules/asyncblock/lib/asyncblock.js:92:26)

I have not yet been able to fix it so opening it up to you guys to have a look in case you have any suggestions.

UPDATE: Works on CircleCI - this was something on my local machine.

mindywhitsitt commented 5 years ago

I'm confused - I seem to be seeing it fail on the sudo apt-get update command, not on any actual test. Maybe I'm not reading the CircleCI output correctly?

ftclausen commented 5 years ago

I'm confused - I seem to be seeing it fail on the sudo apt-get update command, not on any actual test. Maybe I'm not reading the CircleCI output correctly?

Ah, sorry, going by what was happening locally and thought it would get at least past the set up steps here. I'll debug.

mindywhitsitt commented 5 years ago

Maybe once it gets past the setup steps on the build server, then the test will also pass. :)

ftclausen commented 5 years ago

Ah, the issue is our Node is too old and that means the Circle CI image is so old Debian no longer mirrors the files. I'll see if we can just bump to the latest LTS and see if things keep working

ftclausen commented 5 years ago

Maybe once it gets past the setup steps on the build server, then the test will also pass. :)

You were right, once I used current Node LTS + matching container image the build worked fine. The test passes on CircleCI so must be something on my machine.

ftclausen commented 5 years ago

Yeah, I still plan to debug this local issue and we can also see what the other reviewers have to say.

On 26 Jul 2019, at 12:43, Mindy Siota notifications@github.com wrote:

@mindywhitsitt approved this pull request.

Ok cool. Well, it would still be nice to know what's happening on your machine, but passing on the build server sounds good enough for an approval if you ask me (which you did). ;)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.