Skip to content
This repository has been archived by the owner on Dec 26, 2023. It is now read-only.

Add automated testing #3

Open
csnewman opened this issue May 12, 2019 · 6 comments
Open

Add automated testing #3

csnewman opened this issue May 12, 2019 · 6 comments
Labels
testing Related to any CI issues or PR status checks

Comments

@csnewman
Copy link
Contributor

Will allow us to track regressions.

@ed-cooper
Copy link
Owner

I considered this, but I wasn't sure whether it would be worth the time spent refactoring for how little changes will probably be made from this point onwards.

Plus we'd probably need a username and password combination for running tests with? (Unless you mean only doing tests locally with supplied usernames?)

@csnewman
Copy link
Contributor Author

I see two main approaches forward:

  1. A fake http server that will return cached results from the official server (but slightly edited of course)

  2. Separating out the web requests into their own object, and just stubbing that object when testing the rest of the logic

@ed-cooper
Copy link
Owner

The HTML parsing is one of the most important parts of the logic imo, and probably the easiest to break.

Therefore the HTTP server would be the way forwards.

It would be easy enough to introduce variables for the https://login.manchester.ac.uk/ and https://video.manchester.ac.uk/ base URLs, which could then be changed for testing purposes.

But trying to create a semi-accurate web server is more effort than I'd want to do, so I'd be leaving it up to you to implement.

@csnewman
Copy link
Contributor Author

I agree with this course of action.

I would suggest you add the "help-wanted" or/and "up-for-grabs" labels onto this issue

@ed-cooper ed-cooper added help wanted Extra attention is needed testing Related to any CI issues or PR status checks labels May 12, 2019
ed-cooper added a commit that referenced this issue May 12, 2019
@ed-cooper
Copy link
Owner

The login and video URLs are now exposed in the settings, so theoretically setup with a fake HTTP server would now be possible

@ed-cooper
Copy link
Owner

Abstraction of web requests has been completed, now a unit testing setup with dependency injection can be implemented

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
testing Related to any CI issues or PR status checks
Projects
None yet
Development

No branches or pull requests

2 participants