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

Fix [bower] service tests #1383

Merged
merged 1 commit into from
Dec 21, 2017
Merged

Fix [bower] service tests #1383

merged 1 commit into from
Dec 21, 2017

Conversation

RedSparr0w
Copy link
Member

@RedSparr0w RedSparr0w commented Dec 21, 2017

test was previously testing custom labels with ?label="my licence" including the quote marks which is causing tests to fail.
Have updated to exclude the quote marks.

Though not sure why this is suddenly becoming an issue, as it previously has passed with quote marks.
And when i test it on the staging server or shields.io i get the correct response:

But best to remove the quote marks as they are not needed.

1) Bower
       custom label for licence
         
	[ GET http://localhost:1111/bower/l/bootstrap.json?label="my licence" ]:

      AssertionError: expected { name: '"my licence"', value: 'MIT' } to deeply equal { name: 'my licence', value: 'MIT' }
      + expected - actual

       {
      -  "name": "\"my licence\""
      +  "name": "my licence"
         "value": "MIT"
       }
      
      at Object.pathMatch.matchJSON (node_modules/icedfrisby/lib/pathMatch.js:138:38)
      at current.expects.push (node_modules/icedfrisby/lib/icedfrisby.js:724:10)
      at IcedFrisbyNock._invokeExpects (node_modules/icedfrisby/lib/icedfrisby.js:1294:33)
      at start (node_modules/icedfrisby/lib/icedfrisby.js:1274:12)
      at Request.runCallback [as _callback] (node_modules/icedfrisby/lib/icedfrisby.js:1232:16)
      at Request.self.callback (node_modules/request/request.js:186:22)
      at Request.<anonymous> (node_modules/request/request.js:1163:10)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1085:12)
      at endReadableNT (_stream_readable.js:1056:12)
      at _combinedTickCallback (internal/process/next_tick.js:138:11)
      at process._tickDomainCallback (internal/process/next_tick.js:218:9)

@RedSparr0w
Copy link
Member Author

@paulmelnikow Not sure if its just me, but it doesn't look like the Circle CI test ran correctly on this PR, it seems to have just ran the test as a standard test not a PR test.

On all the other PR's if i click the checkmark/cross it comes up with the following:
image
But on this PR it just redirects me to the original test when i pushed the changes and seems to skip danger also.

Any idea what the cause could be?

@RedSparr0w RedSparr0w merged commit b48091a into badges:master Dec 21, 2017
@RedSparr0w
Copy link
Member Author

Latest daily test failed with 17 errors,
Tomorrows should only fail with 14 errors

@paulmelnikow
Copy link
Member

Hmm… is there any chance you have CircleCI set up on your fork? I've seen the same behavior and assumed that is the reason.

In general, CircleCI is bad about communicating to the runtime environment when it's a PR and when it's not. If there's already a build on a ref, it doesn't trigger a new build when a PR is opened.

One workaround suggested by Danger is to only build on PRs, though I think that would prevent the nightly build from running, and also merges to master…

@paulmelnikow
Copy link
Member

P.S. I'm guessing there was a change to a quoting behavior in URL strings, either in request or IcedFrisby.

Thanks for fixing this!

@RedSparr0w
Copy link
Member Author

Yeah looks like i have CircleCI setup in my fork, maybe it would only trigger another rebuild using the PR details if i pushed another change after creating the PR possibly?
May create another PR later just to test.

@paulmelnikow
Copy link
Member

maybe it would only trigger another rebuild using the PR details if i pushed another change after creating the PR possibly?

I think that is true, but am not completely sure. It may depend on which webhook Circle processes first…

@RedSparr0w
Copy link
Member Author

On 1384 i pushed another commit after creating the PR and it seemed to run all the test correctly but failed because i did not have an API token for danger.

I removed RedSparr0w/shields from my CircleCI and all seems to be running correctly now.

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

2 participants