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.

Sign up for GitHub

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

Jump to bottom

Add Azure DevOps tests service [AzureDevOpsTests AzureDevOpsCoverage] #2412

Merged
merged 5 commits into from
Dec 3, 2018
Merged

Add Azure DevOps tests service [AzureDevOpsTests AzureDevOpsCoverage] #2412

merged 5 commits into from
Dec 3, 2018

Conversation

mehmetseckin
Copy link
Contributor

Added a test results badge service for an Azure Pipelines
build using the ResultSummaryByBuild endpoint. Added
basic unit tests for the service.

Implements the badge described in #2411

@mehmetseckin
Add Azure DevOps tests service
33579da 
Added a test results badge service for an Azure Pipelines
build using the ResultSummaryByBuild endpoint. Added
basic unit tests for the service.

Completes #2411
@shields-ci
Copy link

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

Generated by 🚫 dangerJS

@paulmelnikow
Copy link
Member

This pull request introduces 2 alerts when merging 33579da into a524658 - view on LGTM.com

new alerts:

  • 2 for Inefficient regular expression

Comment posted by LGTM.com

@mehmetseckin
Improve result tests
111cfe9 
Added logic to generate only expected regex combinations. This
makes the result tests more strict, and also addresses the lgtm alerts
about ReDos vulnerabilities.
@paulmelnikow paulmelnikow added the service-badge New or updated service badge label Nov 27, 2018
@calebcartwright
Copy link
Member

Just a thought, but should we extract the getLatestBuildId function (perhaps into the helpers file) so that we don't have duplicate definitions in both the coverage and tests classes?

@mehmetseckin
Copy link
Contributor Author

@calebcartwright - That's a really good shout, I wanted to do that initially, but I thought it's best to keep changes to minimum and keep this PR simple.

I can happily update this PR and add the refactoring changes, or we can simply create a new refactoring PR after this is merged.

@mehmetseckin
Fix compact mode tests
7f37988 
Improved regex list construction and added a conditional separator
for compact / default modes.
@mehmetseckin
Extract identical methods to a base class
93b2e25 
Added a base service class and extracted `getLatestBuildId` method
and extended both coverage and tests services from the base class.
Tests should be run for: [AzureDevOpsCoverage] and [AzureDevOpsTests].
@mehmetseckin
Merge branch 'master' of https://github.com/badges/shields
5a6a31f
@paulmelnikow paulmelnikow changed the title [AzureDevOpsTests] Add Azure DevOps tests service Add Azure DevOps tests service [AzureDevOpsTests AzureDevOpsCoverage] Dec 3, 2018
paulmelnikow
paulmelnikow reviewed Dec 3, 2018
View reviewed changes
Copy link
Member

@paulmelnikow paulmelnikow left a comment

Choose a reason for hiding this comment

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

Thanks so much for refactoring this! Beautiful work.

I'm just re-running the tests to include the coverage service which was affected too.

@paulmelnikow paulmelnikow merged commit e8c411b into badges:master Dec 3, 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:

@paulmelnikow
Copy link
Member

Confusingly, half these tests started failing seemingly moments after this was merged. This is failing on the merge commit as well as current master, so it's not related to subsequent changes.

Would you be up for looking into it?

  AzureDevOpsTests
    unknown build definition
      ✓
	[ GET /azuredevops-powershell/azuredevops-powershell/9999.json ] (297ms)
    404 latest build error response
      ✓
	[ GET /azuredevops-powershell/azuredevops-powershell/1.json ] (97ms)
    no build response
      ✓
	[ GET /azuredevops-powershell/azuredevops-powershell/9999.json ]
    no test result summary response
      ✓
	[ GET /azuredevops-powershell/azuredevops-powershell/1.json ]
    invalid test result summary response
      ✓
	[ GET /azuredevops-powershell/azuredevops-powershell/1.json ]
    no tests in test result summary response
      ✓
	[ GET /azuredevops-powershell/azuredevops-powershell/1.json ]
    test status
GET /azuredevops-powershell/azuredevops-powershell/1.json has FAILED with the following response:
      1)
	[ GET /azuredevops-powershell/azuredevops-powershell/1.json ]
    test status on branch
GET /azuredevops-powershell/azuredevops-powershell/1/master.json has FAILED with the following response:
      2)
	[ GET /azuredevops-powershell/azuredevops-powershell/1/master.json ]
    test status with compact message
GET /azuredevops-powershell/azuredevops-powershell/1.json has FAILED with the following response:
      3)
	[ GET /azuredevops-powershell/azuredevops-powershell/1.json ]
    test status with custom labels
GET /azuredevops-powershell/azuredevops-powershell/1.json has FAILED with the following response:
      4)
	[ GET /azuredevops-powershell/azuredevops-powershell/1.json ]
    test status with compact message and custom labels
GET /azuredevops-powershell/azuredevops-powershell/1.json has FAILED with the following response:
      5)
	[ GET /azuredevops-powershell/azuredevops-powershell/1.json ]

@paulmelnikow paulmelnikow mentioned this pull request Dec 3, 2018
Merged
@calebcartwright
Copy link
Member

calebcartwright commented Dec 3, 2018 •
edited
Loading

Adding my two cents here again :)

it looks like those tests that are failing are live/non-mocked tests that are querying this build pipeline and it looks like the latest job that ran for that pipeline earlier today (0.1.30) is now failing on the build/test step which means there are no test results available.

I believe those tests could be fixed either by resolving the build/test issues that exist in that pipeline, and/or updating those unit tests to point to a different pipeline (perhaps a stable/not-in-use pipeline because real may break from time to time)

I'm going to double check the pipelines I used for the unit tests on the coverage service for the same

@paulmelnikow
Copy link
Member

Aha! Good sleuthing. They're returning no tests which doesn't sound like the right message for this scenario. error or build error would be better.

Is it possible to detect this case?

@paulmelnikow
Copy link
Member

Here are the responses in case they're helpful later:

{ count: 1,
  value:
   [ { _links:
        { self:
           { href:
              'https://dev.azure.com/azuredevops-powershell/4d81da9c-b02f-4f9a-8775-c8444d950246/_apis/build/Builds/32' },
          web:
           { href:
              'https://dev.azure.com/azuredevops-powershell/4d81da9c-b02f-4f9a-8775-c8444d950246/_build/results?buildId=32' },
          sourceVersionDisplayUri:
           { href:
              'https://dev.azure.com/azuredevops-powershell/4d81da9c-b02f-4f9a-8775-c8444d950246/_apis/build/builds/32/sources' },
          timeline:
           { href:
              'https://dev.azure.com/azuredevops-powershell/4d81da9c-b02f-4f9a-8775-c8444d950246/_apis/build/builds/32/Timeline' },
          badge:
           { href:
              'https://dev.azure.com/azuredevops-powershell/4d81da9c-b02f-4f9a-8775-c8444d950246/_apis/build/status/1' } },
       properties: {},
       tags: [],
       validationResults: [],
       plans: [ { planId: '5b389dab-c9d6-493a-ab64-404c1fcf393d' } ],
       triggerInfo: {},
       id: 32,
       buildNumber: '0.1.30',
       status: 'completed',
       result: 'failed',
       queueTime: '2018-12-03T15:20:48.1796269Z',
       startTime: '2018-12-03T15:20:59.234572Z',
       finishTime: '2018-12-03T15:22:03.4763836Z',
       url:
        'https://dev.azure.com/azuredevops-powershell/4d81da9c-b02f-4f9a-8775-c8444d950246/_apis/build/Builds/32',
       definition:
        { drafts: [],
          id: 1,
          name: 'azuredevops-powershell-ci',
          url:
           'https://dev.azure.com/azuredevops-powershell/4d81da9c-b02f-4f9a-8775-c8444d950246/_apis/build/Definitions/1?revision=3',
          uri: 'vstfs:///Build/Definition/1',
          path: '\\',
          type: 'build',
          queueStatus: 'enabled',
          revision: 3,
          project:
           { id: '4d81da9c-b02f-4f9a-8775-c8444d950246',
             name: 'azuredevops-powershell',
             description:
              'This is a very simple (and primitive) PowerShell module to interact with the Azure DevOps REST API.\n\nhttps://github.com/mehmetseckin/azuredevops-powershell',
             url:
              'https://dev.azure.com/azuredevops-powershell/_apis/projects/4d81da9c-b02f-4f9a-8775-c8444d950246',
             state: 'wellFormed',
             revision: 15,
             visibility: 'public' } },
       buildNumberRevision: 30,
       project:
        { id: '4d81da9c-b02f-4f9a-8775-c8444d950246',
          name: 'azuredevops-powershell',
          description:
           'This is a very simple (and primitive) PowerShell module to interact with the Azure DevOps REST API.\n\nhttps://github.com/mehmetseckin/azuredevops-powershell',
          url:
           'https://dev.azure.com/azuredevops-powershell/_apis/projects/4d81da9c-b02f-4f9a-8775-c8444d950246',
          state: 'wellFormed',
          revision: 15,
          visibility: 'public' },
       uri: 'vstfs:///Build/Build/32',
       sourceBranch: 'refs/heads/master',
       sourceVersion: '016287ec39a9e4b0ae5ceb6776941e394e14ce4f',
       priority: 'normal',
       reason: 'individualCI',
       requestedFor:
        { displayName: 'GitHub',
          url:
           'https://app.vssps.visualstudio.com/A1b89fde7-4d96-49b6-9337-1ef58fdbb813/_apis/Identities/db890558-d5de-4037-b700-d0e7078cc68a',
          _links:
           { avatar:
              { href:
                 'https://dev.azure.com/azuredevops-powershell/_apis/GraphProfile/MemberAvatars/svc.MWI4OWZkZTctNGQ5Ni00OWI2LTkzMzctMWVmNThmZGJiODEzOkdpdEh1YiBBcHA6NGQ4MWRhOWMtYjAyZi00ZjlhLTg3NzUtYzg0NDRkOTUwMjQ2' } },
          id: 'db890558-d5de-4037-b700-d0e7078cc68a',
          uniqueName: null,
          imageUrl: null,
          descriptor:
           'svc.MWI4OWZkZTctNGQ5Ni00OWI2LTkzMzctMWVmNThmZGJiODEzOkdpdEh1YiBBcHA6NGQ4MWRhOWMtYjAyZi00ZjlhLTg3NzUtYzg0NDRkOTUwMjQ2' },
       requestedBy:
        { displayName: 'GitHub',
          url:
           'https://app.vssps.visualstudio.com/A1b89fde7-4d96-49b6-9337-1ef58fdbb813/_apis/Identities/db890558-d5de-4037-b700-d0e7078cc68a',
          _links:
           { avatar:
              { href:
                 'https://dev.azure.com/azuredevops-powershell/_apis/GraphProfile/MemberAvatars/svc.MWI4OWZkZTctNGQ5Ni00OWI2LTkzMzctMWVmNThmZGJiODEzOkdpdEh1YiBBcHA6NGQ4MWRhOWMtYjAyZi00ZjlhLTg3NzUtYzg0NDRkOTUwMjQ2' } },
          id: 'db890558-d5de-4037-b700-d0e7078cc68a',
          uniqueName: null,
          imageUrl: null,
          descriptor:
           'svc.MWI4OWZkZTctNGQ5Ni00OWI2LTkzMzctMWVmNThmZGJiODEzOkdpdEh1YiBBcHA6NGQ4MWRhOWMtYjAyZi00ZjlhLTg3NzUtYzg0NDRkOTUwMjQ2' },
       lastChangedDate: '2018-12-03T15:22:03.687Z',
       lastChangedBy:
        { displayName: 'Microsoft.VisualStudio.Services.TFS',
          url:
           'https://app.vssps.visualstudio.com/A1b89fde7-4d96-49b6-9337-1ef58fdbb813/_apis/Identities/00000002-0000-8888-8000-000000000000',
          _links:
           { avatar:
              { href:
                 'https://dev.azure.com/azuredevops-powershell/_apis/GraphProfile/MemberAvatars/s2s.MDAwMDAwMDItMDAwMC04ODg4LTgwMDAtMDAwMDAwMDAwMDAwQDJjODk1OTA4LTA0ZTAtNDk1Mi04OWZkLTU0YjAwNDZkNjI4OA' } },
          id: '00000002-0000-8888-8000-000000000000',
          uniqueName: null,
          imageUrl: null,
          descriptor:
           's2s.MDAwMDAwMDItMDAwMC04ODg4LTgwMDAtMDAwMDAwMDAwMDAwQDJjODk1OTA4LTA0ZTAtNDk1Mi04OWZkLTU0YjAwNDZkNjI4OA' },
       orchestrationPlan: { planId: '5b389dab-c9d6-493a-ab64-404c1fcf393d' },
       logs:
        { id: 0,
          type: 'Container',
          url:
           'https://dev.azure.com/azuredevops-powershell/4d81da9c-b02f-4f9a-8775-c8444d950246/_apis/build/builds/32/logs' },
       repository:
        { id: 'mehmetseckin/azuredevops-powershell',
          type: 'GitHub',
          clean: null,
          checkoutSubmodules: false },
       keepForever: false,
       retainedByRelease: false,
       triggeredByBuild: null } ] }
{ aggregatedResultsAnalysis:
   { previousContext: { contextType: 0, build: null, release: null },
     resultsDifference:
      { increaseInTotalTests: 0,
        increaseInFailures: 0,
        increaseInPassedTests: 0,
        increaseInOtherTests: 0,
        increaseInDuration: '00:00:00' },
     totalTests: 0,
     duration: '00:00:00',
     resultsByOutcome: {},
     runSummaryByState: {},
     runSummaryByOutcome: {} },
  testResultsContext:
   { contextType: 'build',
     build: { id: 32, definitionId: 0, uri: 'vstfs:///Build/Build/32' },
     release: null },
  teamProject:
   { id: '4d81da9c-b02f-4f9a-8775-c8444d950246',
     name: 'azuredevops-powershell',
     state: 'unchanged',
     visibility: 'unchanged' } }

@calebcartwright
Copy link
Member

I believe no tests is the expected message, at least as far as that Azure DevOps API is concerned.

If a job runs (regardless of whether it passes or fails) that doesn't execute or publish test results, then there's just no test results associated with that particular job. It just so happens that in this particular instance there's no test results published because there is a failure earlier in the pipeline. It's now the equivalent of a job with 0 test results.

The only way I could think of to check for this scenario would be to also review the build status in the API call sequence. However, I'm not sure we'd want to do that as test results could be available even when the job failed. Scenario: I could have a build pipeline that runs my tests, generates code coverage results, and publishes both reports successfully, but I could also have another step in my pipeline that fails the job (for example a code coverage enforcement step that will fail the job if coverage dips below X%). In that instance I'd still want to have my test results & coverage badges available, even though that latest job failed

@mehmetseckin
Copy link
Contributor Author

Wow, I didn't expect that to happen 😄 thanks for following up!

I agree with @calebcartwright, no tests is the expected message in case of an empty response, and there is other unit tests to assert the message in that case.

In those failed tests, I intended to assert the message produced by the badge when there are some tests available. I think mocking the API calls here and providing a preset response would be a better approach, I think we shouldn't be testing the live API itself.

I'm happy to refactor these tests and add mock responses.

@mehmetseckin mehmetseckin mentioned this pull request Dec 4, 2018
Merged
@paulmelnikow
Copy link
Member

Thanks so much! Let's continue discussion at #2454 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulmelnikow paulmelnikow paulmelnikow approved these changes

Assignees
No one assigned
Labels
service-badge New or updated service badge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mehmetseckin @shields-ci @paulmelnikow @calebcartwright