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

Feat/training coach debug #232

Merged
merged 13 commits into from
Dec 18, 2024
Merged

Feat/training coach debug #232

merged 13 commits into from
Dec 18, 2024

Conversation

tercierp
Copy link
Collaborator

@tercierp tercierp commented Dec 17, 2024 •
edited
Loading

Overview

The purpose of this PR is to solve all the testing problems that existed with the CI.

  • I've added tests for coverage
  • Correct some omissions
  • Control CI behaviors (test to launch the CI by deleting the cache, it's a solution for certain tests that fail!)

@tercierp tercierp added this to the Milestone M3 milestone Dec 17, 2024
@tercierp tercierp self-assigned this Dec 17, 2024
This was referenced Dec 17, 2024
Closed
Closed
@tercierp tercierp changed the base branch from main to feat/training-coach-frontend-final December 17, 2024 22:14
@tercierp tercierp added Bug 🐛 Something isn't working and removed Fixed bug ✅ labels Dec 18, 2024
@tercierp tercierp marked this pull request as ready for review December 18, 2024 15:10
@tercierp tercierp linked an issue Dec 18, 2024 that may be closed by this pull request
Patch the MVVM workout #222
Closed
SaturneV
SaturneV approved these changes Dec 18, 2024
View reviewed changes
Copy link
Contributor

@SaturneV SaturneV left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great fixes, I just left you a little suggestion that I'll let you read and decide what to do with it. I already approve the PR. Good job!

app/src/androidTest/java/com/android/streetworkapp/ui/train/TrainHubScreenTest.kt
// Verify ConfirmButton
composeTestRule.onNodeWithTag("ConfirmButton").assertExists()
// composeTestRule.onNodeWithTag("ConfirmButton").assertExists()
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you could just add a comment about why you're commenting on this verification so that the person reading the test has more context.

Copy link
Collaborator Author

@tercierp tercierp Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have no concrete explanation as to why this assertion needs to be commented on, it's possible that the emulator doesn't load this button. However, the button is in a LazyColumn with all the other components, I think I'll investigate in my e2e fix.

Is it enough if I put a comment : //TODO check this in the E2E training debug branch ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation. Sure then it's a good idea to add that!

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@tercierp tercierp merged commit 8b411ab into feat/training-coach-frontend-final Dec 18, 2024
3 checks passed
@tercierp tercierp deleted the feat/training-coach-debug branch December 18, 2024 16:29
@tercierp tercierp mentioned this pull request Dec 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SaturneV SaturneV SaturneV approved these changes

Assignees

@tercierp tercierp

Labels
Bug 🐛 Something isn't working Fixed bug ✅
Projects
None yet
Milestone
Milestone M3
Development

Successfully merging this pull request may close these issues.

Patch the MVVM workout
2 participants
@tercierp @SaturneV