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

Php tests and error boundaries in react router #2140

Draft
wants to merge 37 commits into
base: legacy
Choose a base branch
from
Draft

Php tests and error boundaries in react router #2140

wants to merge 37 commits into from

Conversation

jaltekruse
Copy link
Contributor

  • this is based on top of the branch that adds Jest, so comments on those first couple of commits can be directed to the dedicated PR for that - Add jest for javascript testing #2138
  • I want to add tests for the error boundaries, but we won't necessarily always be able to manipulate the back-end to throw an error for any request using the tools accessible from cypress. We still want to test we have defensive code in javascript if unexpected things happen, like the database being temporarily down, future code changes cause bugs, etc.
  • So I wanted to have both the error boundaries code together with adding javascript unit tests, as I think we will likely need to have some off the test coverage for handling errors to be just in javascript and simulating error responses from the server, I'll be adding those tests soon
  • I still need to do a little cleanup on this branch before it is ready for merging, but I wanted to get up the example of a php based test for an API

jaltekruse and others added 30 commits May 15, 2023 11:21
@jaltekruse
WIP - cypress level test
4f05577
@jaltekruse
getting jest set up
dd0fe03
@jaltekruse
WIP - testing stuff
e8aeacf
@jaltekruse
WIP - trying to get a test running with react-router and recoil stuff…
fcb31e7 
… together

- current error - TypeError: Cannot read properties of undefined (reading 'subscribe')
@jaltekruse
First working test with react-router
8b06564 
- I forgot to stage the right file last commit, so I didn't save my in-progress state, but I did end up copying it to start with the simpler version and fix it
@jaltekruse
Factoring out a little bit of code
ae682c5
@jaltekruse
Make the utility function do a little more, delete the bad version of…
971d8fa 
… the test
@jaltekruse
Small cleanups, delete incomplete test for some recoil code
00e685f
@jaltekruse
Rename test, add vscode config for debugging jest tests, remove incom…
e5ca25a 
…plete cypress test
@jaltekruse
revert changes to courseActions
c1b2cdd 
- was a minor fix to get the code to run from a unit test, will
  worry about this later when I finish that test and fix the bug
@jaltekruse
Add an extension for jest to the devcontainer
fbb26f0
@jaltekruse
Update package-lock.json
40c0125
@jaltekruse
change include to include_once to make testing easier
0e81be6
@jaltekruse
Add base model to abstract away common SQL boilerplate
2f9a987
@jaltekruse
Move to using try/catch and the query
b9db3bc
@jaltekruse
Create utility function for checking for required inputs
32fcdab
@jaltekruse
Adding an ErrorPanel component that can display loader error automati…
1911d86 
…cally, must use axios requests! don't use fetch()
@jaltekruse
temp - throw an error
8e09989
@jaltekruse
This is the wrong way to do things, makes the whole page break on one…
caad906 
… failed network request
@jaltekruse
This does what I want, seems a bit weird to have a child route that s…
3dc259b 
…hares a route with the parent
@jaltekruse
Fixing admin page to use axios
6f1bbae
@jaltekruse
Don't give error on no promoted content groups (maybe revert?)
e9f9f71
@jaltekruse
Fix the community page
10a4660
@jaltekruse
Fix bug, remove leftover code from previously using fetch()
5b7d294
@Lyanthropos @jaltekruse
add php to dev container
11471cc
@jaltekruse
WIP - tests with phpunit
267525e
@jaltekruse
Start factoring out common queries into views
dcb831f
@jaltekruse
Can't put the nullish coalesce here because different fields need to …
2185066 
…be treated differently
@jaltekruse
Move the coalescing into SQL, this way generic boolean handling when …
69a3820 
…reading results from SQL is safe, TODO still need to cast to tinyint to make it clear what is a boolean
@jaltekruse
Refactor to use try/catch
d7b81bb
jaltekruse added 7 commits May 16, 2023 18:15
@jaltekruse
Use convenience method for all sql queries to ensure we are always ch…
933f645 
…ecking for SQL errors, remove some boilerplate
@jaltekruse
Getting enough test infrastructure in place so we can call APIs
9b3ddc6 
- had to implement a little workaround for capturing script output, as PHPUnit has some
  functionality around this, but it is a bit restrictive. You cannot include/call more than one
  script while also wanting to assert on the output of any of the scripts, as it doesn't include
  an API to clear the output buffer, just to access the current contents, which can still be
  appended to by later code run during the test. This also include test setup code.
@jaltekruse
Got an item added to a course in a test
1ebe52d
@jaltekruse
Fix namespacing when referring to things in global namespace
14561f4
@jaltekruse
Add some more assertions
0d16230
@jaltekruse
Fix an issue in PHP now that we are adding a namespace to allow mocki…
bb2c295 
…ng in tests
@jaltekruse
Removing extra error added for demo purposes
7737059
@jaltekruse jaltekruse marked this pull request as draft May 17, 2023 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevincharles kevincharles Awaiting requested review from kevincharles

@dqnykamp dqnykamp Awaiting requested review from dqnykamp

@Lyanthropos Lyanthropos Awaiting requested review from Lyanthropos

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jaltekruse @Lyanthropos