Skip to content

Print web link#683

Merged
pawelpasterz merged 3 commits into
masterfrom
print_web_link
Apr 1, 2020
Merged

Print web link#683
pawelpasterz merged 3 commits into
masterfrom
print_web_link

Conversation

@bootstraponline
Copy link
Copy Markdown
Contributor

@bootstraponline bootstraponline commented Mar 25, 2020

Fixes #645

Checklist

  • Unit tested
  • release_notes.md updated
  4 matrix ids created in 0m 22s
  https://console.developers.google.com/storage/browser/test-lab-v9cn46bb990nx-kz69ymd4nm9aq/2020-04-01_07-25-03.132000_AQWv/

Matrices webLink
  matrix-26tm1277rg3gr: https://console.firebase.google.com/project/flank-open-source/testlab/histories/bh.da0c237aaa33732/matrices/8849499259998194241
  matrix-2veeo7xug1f8k: https://console.firebase.google.com/project/flank-open-source/testlab/histories/bh.da0c237aaa33732/matrices/5106500884348802256
  matrix-2z6o0hu8tt5bf: https://console.firebase.google.com/project/flank-open-source/testlab/histories/bh.da0c237aaa33732/matrices/4681223845649672500
  matrix-2jl1g9zptcs9m: https://console.firebase.google.com/project/flank-open-source/testlab/histories/bh.da0c237aaa33732/matrices/6721963817674217321/executions/bs.faacc05c891cc43c

PollMatrices

@bootstraponline bootstraponline force-pushed the print_web_link branch 3 times, most recently from 4388082 to d91f778 Compare March 25, 2020 05:29
@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 25, 2020

Codecov Report

Merging #683 into master will increase coverage by 0.07%.
The diff coverage is 77.77%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #683      +/-   ##
============================================
+ Coverage     77.26%   77.33%   +0.07%     
  Complexity      633      633              
============================================
  Files           110      110              
  Lines          2555     2563       +8     
  Branches        364      365       +1     
============================================
+ Hits           1974     1982       +8     
+ Misses          348      347       -1     
- Partials        233      234       +1     

if (printWebLink && webLink.isNotBlank()) {
printWebLink = false
println(webLink)
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is there a cleaner way to print the web link once per matrix?

Copy link
Copy Markdown
Contributor

@pawelpasterz pawelpasterz Mar 25, 2020

Choose a reason for hiding this comment

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

I am wondering if we could print this link using this logic in SavedMatrix

if (changedLink) {
this.webLink = newLink
}

AFAIK link is updated only once for each matrix (please confirm that, I've tested it with couple of examples but might be some corner cases about which I am not aware of).
Also, I think we could print it in more user friendly way -- with indention, message saying what exactly this is etc.
Just thinking, let me know what you think.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I like that idea!

My understanding is the matrix link is only updated once. I'll update the pull request.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we need to print web link ASAP "after starting run", I guess that the cleaner way is to do first refresh of matrices somewhere here. So the web link can be properly generated and printed before "PollMatrices" message will appears. But it's not good idea if number of additional pulls matters.

@pawelpasterz pawelpasterz requested a review from jan-goral April 1, 2020 07:30
@pawelpasterz pawelpasterz self-assigned this Apr 1, 2020
jan-goral
jan-goral previously approved these changes Apr 1, 2020
Copy link
Copy Markdown
Contributor

@jan-goral jan-goral left a comment

Choose a reason for hiding this comment

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

Great job!

bootstraponline and others added 3 commits April 1, 2020 10:49
PollMatrices
  0m  0s matrix-1h3w820q84crs NexusLowRes-28 VALIDATING
  0m  0s matrix-1h3w820q84crs NexusLowRes-28 VALIDATING
https://console.firebase.google.com/project/flank-open-source/testlab/histories/bh.bf178d418be9d33e/matrices/6081298694548031160
  0m  5s matrix-1h3w820q84crs NexusLowRes-28 PENDING
@pawelpasterz pawelpasterz merged commit aa6d9b8 into master Apr 1, 2020
@pawelpasterz pawelpasterz deleted the print_web_link branch April 1, 2020 08:56

private tailrec suspend fun getOrUpdateWebLink(link: String, project: String, matrixId: String): String =
if (link.isNotBlank()) link
else getOrUpdateWebLink(GcTestMatrix.refresh(matrixId, project).webLink(), project, matrixId)
Copy link
Copy Markdown
Contributor Author

@bootstraponline bootstraponline Apr 1, 2020

Choose a reason for hiding this comment

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

A matrix is not guaranteed to have a webLink. A matrix can be invalid.
https://github.com/Flank/flank/blob/master/test_runner/src/main/kotlin/ftl/util/MatrixState.kt

Here are the possible states:

        "enumDescriptions": [
            "Do not use.  For proto versioning only.",
            "The execution or matrix is being validated.",
            "The execution or matrix is waiting for resources to become available.",
            "The execution is currently being processed.\n\nCan only be set on an execution.",
            "The execution or matrix has terminated normally.\n\nOn a matrix this means that the matrix level processing completed normally,\nbut individual executions may be in an ERROR state.",
            "The execution or matrix has stopped because it encountered an\ninfrastructure failure.",
            "The execution was not run because it corresponds to a unsupported\nenvironment.\n\nCan only be set on an execution.",
            "The execution was not run because the provided inputs are incompatible with\nthe requested environment.\n\nExample: requested AndroidVersion is lower than APK's minSdkVersion\n\nCan only be set on an execution.",
            "The execution was not run because the provided inputs are incompatible with\nthe requested architecture.\n\nExample: requested device does not support running the native code in\nthe supplied APK\n\nCan only be set on an execution.",
            "The user cancelled the execution.\n\nCan only be set on an execution.",
            "The execution or matrix was not run because the provided inputs are not\nvalid.\n\nExamples: input file is not of the expected type, is malformed\/corrupt, or\nwas flagged as malware"
          ],
          "type": "string",
          "enum": [
            "TEST_STATE_UNSPECIFIED",
            "VALIDATING",
            "PENDING",
            "RUNNING",
            "FINISHED",
            "ERROR",
            "UNSUPPORTED_ENVIRONMENT",
            "INCOMPATIBLE_ENVIRONMENT",
            "INCOMPATIBLE_ARCHITECTURE",
            "CANCELLED",
            "INVALID"
          ]

I think only:

"VALIDATING",
"PENDING",
"RUNNING",
"FINISHED",

will possibly have a webLink that we can poll for.

"TEST_STATE_UNSPECIFIED",
"UNSUPPORTED_ENVIRONMENT",
"INCOMPATIBLE_ENVIRONMENT",
"INCOMPATIBLE_ARCHITECTURE",
"CANCELLED",
"INVALID"

I don't think we'll get a web link from these states. In the current implementation, will we poll indefinitely until a web link is provided?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for input. I will implement additional logic to prevent this.

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.

Print test results URL

4 participants