Conversation
|
Lots of tests don't run locally for me, so I'm having trouble recreating things locally. Do they run fine for you? |
Yes they do. |
There was a problem hiding this comment.
There's a lot of formatting changes in this file. If they're not needed, can we restore the original version please?
There was a problem hiding this comment.
Yamllint
There was a problem hiding this comment.
What do you mean? Yamllint doesn't cause this, it didn't complain about the previous formatting. I think it must be an issue with your IDE formatting settings.
There was a problem hiding this comment.
We have more of this in end-to-end-tests-all-devices.yaml and run-end-to-end-tests/action.yml
There was a problem hiding this comment.
I don't have any additional settings for yaml linting on my IDE (just the ones on the repo).
There was a problem hiding this comment.
I can see in the linked job that there was an issue with line length on one line.
It's good that you fixed that, but that doesn't mean we need to split all variable declarations to two lines using a | or that we need to replace all ' with ". Those are my main concerns, as they show up as changes in the diff and makes it hard to read and figure out what the actual changes are.
chinmaymudholkar1
force-pushed
the
macos-vm
branch
from
f982a68 to
412d1f4
Compare
Pull request was converted to draft
| # WebKit may open CSV in browser - read from <pre> element | ||
| pages = self.page.context.pages | ||
| page_to_read = pages[-1] if len(pages) > 1 else self.page | ||
| page_to_read.wait_for_load_state("load", timeout=page_load_timeout) |
There was a problem hiding this comment.
Do we really need this wait? We already waited 5s in the try block before we enter the except block.
| - device: iPhone 15 | ||
| os: macos-latest | ||
| cache_path: ~/Library/Caches/ms-playwright | ||
| max-parallel: 5 |
There was a problem hiding this comment.
I can see that you changed it to 2 in order to
avoid teams getting deleted
But now it's back up to 5 again. If this can truly cause flakiness, should we not just turn it down to 1? It seems that the workflow doesn't take that long, and it mainly runs as night, so it should probably be fine?
There was a problem hiding this comment.
I am seeing a strange 500 response on the reset_team function. This is only happening on GH, not on local. I have tried various solutions on different commits, but the issue persists. Even 1 parallel run does not help.
| { | ||
| echo "tests=" | ||
| echo "cross_service_tests=true" | ||
| echo "cross_service_tests=false" |
There was a problem hiding this comment.
Why are we turning this off?
There was a problem hiding this comment.
I am seeing a strange 500 response on the reset_team function. This is only happening on GH, not on local. I have tried various solutions on different commits, but the issue persists. Even 1 parallel run does not help.
There was a problem hiding this comment.
Hmm, I can see there's lots of tests failing (I reran them too). But it seems like they're failing even with this being off, so I don't think we need to turn these tests off to solve it, it doesn't seem related.
| imms_api_tests: "false" | ||
| pds_api_tests: "false" | ||
| reporting_tests: "false" |
There was a problem hiding this comment.
Again, why are we turning these off?
There was a problem hiding this comment.
I am seeing a strange 500 response on the reset_team function. This is only happening on GH, not on local. I have tried various solutions on different commits, but the issue persists. Even 1 parallel run does not help.
|
This needs more thought. I will reattempt this later. |
2 participants
Tests running on Safari and mobile devices regularly fail due to two main issues:
Changes in this PR fix these issues: