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 [suggest] #2604

Merged
merged 1 commit into from
Dec 28, 2018
Merged

Fix [suggest] #2604

merged 1 commit into from
Dec 28, 2018

Conversation

paulmelnikow
Copy link
Member

@paulmelnikow paulmelnikow added frontend The Docusaurus app serving the docs site bug Bugs in badges and the frontend labels Dec 28, 2018
@shields-ci
Copy link

Warnings
⚠️

This PR modified helper functions in lib/ but not accompanying tests.
That's okay so long as it's refactoring existing code.

Messages
📖 ✨ Thanks for your contribution to Shields, @paulmelnikow!

Generated by 🚫 dangerJS against df25272

@paulmelnikow paulmelnikow merged commit 16491d7 into master Dec 28, 2018
@shields-deployment
Copy link

This pull request was merged to master branch. This change is now waiting for deployment, which will usually happen within a few days. Stay tuned by joining our #ops channel on Discord!

After deployment, changes are copied to gh-pages branch:

@techtonik
Copy link
Contributor

Is it possible to have a smoke test for this functionality?

@paulmelnikow
Copy link
Member Author

It gets tested as part of the service test run. The test was failing, but hadn’t been picked up on yet.

@techtonik
Copy link
Contributor

I don't see any failures in https://github.com/badges/shields/commits/master - is service test run is a manual process?

@paulmelnikow
Copy link
Member Author

The full service-test run is here:

https://circleci.com/gh/badges/daily-tests

The suggest tests will be passing now because the fix is merged.

We can’t run all the tests on every commit so we run affected tests as parts of PRs. The brackets around suggest in the PR indicate that test suite ran here.

Occasionally a change will affect something unexpected so it gets caught later by the daily tests.

@techtonik
Copy link
Contributor

We can’t run all the tests on every commit

Why? It is only 4 minutes in the daily log.

@paulmelnikow
Copy link
Member Author

Our PR build runs in about 20 seconds; 4 minutes is much too slow. The other issue is that the service tests rely on external services. #927 is the discussion about that. We used to run the full suite on every commit to master, but stopped because they often fail for reasons outside our control. The thing we could use more help with is keeping up with fixing the failures identified tests (i.e. #1359).

@paulmelnikow paulmelnikow deleted the fix-suggest-2 branch December 31, 2018 02:45
@techtonik
Copy link
Contributor

I see. Is there a way to make a "yellow" badge status if tests are failed because of external services and resort to "red" if there is something wrong with the code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs in badges and the frontend frontend The Docusaurus app serving the docs site
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants