fix(app): show correct git status for newly created/modified files#20744
fix(app): show correct git status for newly created/modified files#20744lmiller1990 merged 31 commits into10.0-releasefrom
Conversation
|
Thanks for taking the time to open a PR!
|
Test summaryRun details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
||||||||||||||||||||||||||
| import { createTestDataContext, scaffoldMigrationProject } from '../helper' | ||
|
|
||
| describe('GitDataSource', () => { | ||
| it(`gets correct status for files on ${os.platform()}`, async () => { |
There was a problem hiding this comment.
Making sure it runs on windows/linux/macos.
4c9178b to
372f78a
Compare
marktnoonan
left a comment
There was a problem hiding this comment.
I've done a bit of testing and seems to work great overall. There are two things to look at. First, when creating a spec from Cypress, it doesn't show the git status in the table, see filename.cy.ts here:
Second, in the mocks, before files are committed, the designs show "created" and "modified" without the name attached. Should we match that here?
Love the new tests!
| // macOS/Linux: getInfoPosix | ||
| // Windows: getInfoWindows | ||
| // Where possible, we use SimpleGit to get other git info, like | ||
| // the status of untracked files and the current git username. |
There was a problem hiding this comment.
Really happy to have this reasoning included
| @@ -0,0 +1 @@ | |||
| export const gitStatusType = ['modified', 'created', 'unmodified'] as const | |||
There was a problem hiding this comment.
It's almost like we should have this model of files for everything in the constants.ts file.
I like this though.
|
@marktnoonan I have
I put a video demo here to show the new UI and it working as expected: https://cypress-io.atlassian.net/browse/UNIFY-863?focusedCommentId=17183 This was a surprisingly complex ticket. Our virtualized spec list is a bit weird (well, how we compose Anyway, it works for now! Please re-review. cc @elevatebart for one more ✔️, thanks! |
I have comments but am OOO today, so don't want my review to be blocking
|
@lmiller1990 I'm confused by this Percy snapshot, and this one. The first seems to show results where we should be on the empty state, and both show I'd agree about revisiting the specs list, @ZachJW34 and I have talked about this as well. There are a couple of related problems it would be good to talk about together. |
|
Should foo.cy.js and blah.cy.js have been checked in? It looks like they're generated as part of a test |
|
@marktnoonan thanks for catching this. I'd forgotten to update the stubs, in this file. Happy to wait for your re-review if you are out, or get someone else to 👀. I don't really understand why the generated mock data has changed, and I don't know how to get it back to the original state. We might just need to re-approve these. Far too much time has been wasted on these dynamic fixtures, I'm going to remove them. Ticket: https://cypress-io.atlassian.net/browse/UNIFY-1403. @ryanthemanuel thanks, I removed those two junk testing files. @elevatebart Can you please re-review? |

User facing changelog
/bin/sh; default to$SHELLor/bin/bashfor unix-like systems).Additional details
N/A
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?cypress.schema.json?