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.

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

🧪 Add end-to-end tests for navigation #4013

Merged
merged 29 commits into from
Jan 25, 2023
Merged

🧪 Add end-to-end tests for navigation #4013

merged 29 commits into from
Jan 25, 2023

Conversation

rye
Copy link
Member

@rye rye commented Sep 15, 2019

I like testing.

Testing is good.

"We should test more stuff" has been a long-standing thing I've said, but here I'm actually trying my hand at e2e testing.

I'm Not Sure™ if this affects coverage at all. It'd be nice if it did, but I don't think Detox instruments that yet, or if they do, I don't think we're uploading that. So, I guess I just have to give this a try.


list of views to test moved to #6788

@rye rye added tool/detox testing related to testing labels Sep 15, 2019
@rye rye self-assigned this Sep 15, 2019
@rye rye changed the title Add end-to-end tests for navigation 🧪 Add end-to-end tests for navigation Sep 15, 2019
e2e/streaming.spec.js Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 15, 2019

Codecov Report

Merging #4013 (f64c56d) into master (c2358b2) will not change coverage.
The diff coverage is 0.00%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #4013   +/-   ##
======================================
  Coverage    8.59%   8.59%           
======================================
  Files         298     298           
  Lines        5097    5097           
  Branches     1383    1383           
======================================
  Hits          438     438           
  Misses       4636    4636           
  Partials       23      23           

@hawkrives
Copy link
Member

I think we should tweak the Detox tests to not reinstall the app before every test. I don’t remember why I did that originally.

@hawkrives
Copy link
Member

Anyway, ideas:

  • Calendar, tap first item in the list of there are any items

  • Building Hours, same thing

  • Settings, assert that the Sign In button is visible?

Also, instead of deleting and reinstalling the app before each
individual test, (which is overkill) just launch a new instance of the
app.

Signed-off-by: Kristofer Rye <[email protected]>
@rye
Copy link
Member Author

rye commented Sep 15, 2019

I have taken care of the relaunching behavior in 6dd0d1f.

@rye
Copy link
Member Author

rye commented Sep 15, 2019

So I can't find my testID because "React Native only supports the testID prop on the native built-in components."

It sounds like I have to manually propagate that through to the appropriate element.

rye added 2 commits September 15, 2019 13:31
Signed-off-by: Kristofer Rye <[email protected]>
Since screen-streaming is actually just a TabNavigator, and I can't
quite figure out how to set the testID on it, I'm going to ignore it
and see if I can get a passing build

Signed-off-by: Kristofer Rye <[email protected]>
@rye
Copy link
Member Author

rye commented Sep 15, 2019

Hey, a passing build!

@drewvolz
Copy link
Member

List of views I think we could add E2E tests to. Feel free to edit.

Home

  • List of buttons

Menus

  • Stav Hall
  • Cage
  • Pause
  • Burton
  • LDC
  • Weitz
  • Sayles
  • Menu Item Detail

SIS

  • Balances
  • Course Search Home
  • Course Search List
  • Course Search Detail
  • TES
  • Open Jobs List
  • Open Jobs Detail

Building hours

  • Hours List
  • Hours Detail
  • Hours Problem Report
  • Hours Schedule Editor

Calendar

  • St. Olaf Calendar
  • Oleville Calendar
  • Northfield Calendar
  • Calendar Event Detail

Directory

  • Link to browser (current implementation)
  • Directory List (awaiting PR merge)
  • Directory Contact Detail (awaiting PR merge)

Streaming Media

  • Events calendar
  • Webcams
  • KSTO Radio
  • KRLX Radio
  • Radio Schedule

News

  • St. Olaf List
  • Oleville List
  • Mess List
  • PoliticOle List
  • KSTO List

Map

  • Link to Browser

Important Contacts

  • Contacts List
  • Contacts Detail

Transit

  • Express Bus
  • Red Line
  • Blue Line
  • Oles Go
  • Other Modes
  • Other Modes Detail

Dictionary

  • Dictionary List
  • Dictionary Detail
  • Dictionary Editor

Student Orgs

  • Orgs List
  • Orgs Detail

Moodle

  • Link to Browser

Report a Problem

  • List of help cards

stoPrint

  • Printer List
  • Print Job Release

Safety Concerns

  • Link to Browser

Settings

  • Login
  • Contact Us
  • FAQs
  • Reset
  • Icon Settings
  • Credits
  • Privacy Policy
  • Legal
  • Contributing (link to browser)

Smaller Fish

Developer

  • API Test
  • BonApp Picker
  • Redux Debug
  • Sentry Message
  • Sentry Exception

Integrated but not visible

  • Bus Map

rye added 4 commits October 13, 2019 15:31
I am doing this primarily to get the up-to-date GitHub Actions workflows
and such, but also the changes that have been made to various views.

Signed-off-by: Kristofer Rye <[email protected]>
@drewvolz
Copy link
Member

I realize these have been sitting in this PR but probably can be merged in as they are written. Is there anything besides resolving conflicts that we need to do to get these in?

@hawkrives
Copy link
Member

Riffing from @drewvolz's comment, I think that once we get Detox working again this should be only a small amount of work to merge!

* master: (2979 commits)
  update Podfile.lock
  update theme import and change names
  use standard "black" color
  switch codecov from the bash uploader to their action
  add new "dependency review" workflow
  add whitespace to "check" workflow jobs
  make the Prettier job's ID match its name
  upgrade linux runners to ubuntu-22.04
  centralize global versions for languages and Xcode
  auto-cancel in-progress jobs on subsequent commit to the same branch
  add distributionSha256Sum to gradle-wrapper.properties
  implement auto-cache-cleanup workflow
  Bump react-native-paper from 4.12.4 to 5.1.4
  Bump babel-jest from 29.3.1 to 29.4.0
  Bump @jest/globals from 29.3.1 to 29.4.0
  Bump @typescript-eslint/parser from 5.48.2 to 5.49.0
  Bump react-native-network-logger from 1.13.0 to 1.14.0
  Bump @typescript-eslint/eslint-plugin from 5.48.2 to 5.49.0
  Bump jest from 29.3.1 to 29.4.0
  Bump @react-native-community/datetimepicker from 6.7.2 to 6.7.3
  ...
@hawkrives hawkrives marked this pull request as ready for review January 24, 2023 16:09
@hawkrives hawkrives requested a review from drewvolz as a code owner January 24, 2023 16:09
@hawkrives hawkrives marked this pull request as draft January 24, 2023 16:09
@hawkrives
Copy link
Member

  ● Streaming Media View › has the Stream List visible by default

    Test Failed: No elements found for “MATCHER(id == “homescreen-button-StreamingView”)”

    HINT: To print view hierarchy on failed actions/matches, use log-level verbose or higher.

      30 | 	it('has the Stream List visible by default', async () => {
      31 | 		// Navigate into StreamingView
    > 32 | 		await element(by.id('homescreen-button-StreamingView')).tap()
         | 		                                                        ^
      33 |
      34 | 		// The stream-list should be visible now
      35 | 		await expect(element(by.id('stream-list'))).toBeVisible()

      at Object.<anonymous> (e2e/streaming.spec.ts:32:59)

Working on this part next…

@hawkrives hawkrives marked this pull request as ready for review January 24, 2023 22:57
@hawkrives
Copy link
Member

ready!

Copy link
Member Author

@rye rye left a comment

Choose a reason for hiding this comment

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

I can't ✅ this but I love it! This certainly seems to be going in the right direction.

As to overall strategy, I do think it's worth us exploring either mocking or full parametrization of things so that we have more control over data we expect to see in the app from the test suite and so that we can verify more tightly. However, I think we should do that in a separate MR — I think that's part of the larger infrastructurification push.

@hawkrives
Copy link
Member

I fully agree! And I agree about that being a separate effort - we'll get a lot of value just from having the assurance that every view is reachable by navigating the app, I think!

@hawkrives hawkrives merged commit 5101d26 into master Jan 25, 2023
@hawkrives hawkrives deleted the more-testing branch January 25, 2023 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing related to testing tool/detox
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants