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

[Discuss] How to improve testing #933

Closed
6 of 8 tasks
tunetheweb opened this issue Jun 28, 2020 · 21 comments
Closed
6 of 8 tasks

[Discuss] How to improve testing #933

tunetheweb opened this issue Jun 28, 2020 · 21 comments
Labels
development Building the Almanac tech stack good first issue Good for newcomers question Further information is requested

Comments

@tunetheweb
Copy link
Member

tunetheweb commented Jun 28, 2020

As discussed in the Goals section of the Developer's Guide, I'd like to consider how we can increase our automatic testing.

The tech stack is starting to get complicated and with 2020 coming along, we should look at how we can ensure we don't break old functionality when adding new functionality.

Thoughts so far include:

Any other thoughts @HTTPArchive/developers ?

@tunetheweb tunetheweb added question Further information is requested development Building the Almanac tech stack labels Jun 28, 2020
@tunetheweb tunetheweb added the good first issue Good for newcomers label Jun 28, 2020
@ibnesayeed
Copy link
Contributor

Adding a Dockerfile with necessary development and testing environment will help contributors test their changes locally more easily before pushing.

@tunetheweb
Copy link
Member Author

I've not used Docker much to be honest. What would that give us? Just conscious of adding extra tech and complexity so would want to know the benefits and how we maintain the docker file going forward.

At the moment you need node v12, and Python 3 installed (which are standard enough development tools that I would expect most developers to have installed IMHO). Then we do have some more requirements to install though npm install and pip (as detailed in src/README.md). Those dependencies do take a few minutes to install admittedly, but they are one command to run, and only need to be done once. It's then possible to run the site in MacOS, Windows and Linux (not tried linux, but presume so) and developers can develop locally.

I'm guessing Docker could help with those dependency installs? But not sure if installing Docker would take just as long for those of us that don't use it, and would give us another thing to maintain?

What I think we need is a combination of:

  • Test scripts - like pytest that we have, to run though some test cases. Ideally developers should do this before submitting PRs, and we should do again when releasing.
  • GitHub actions - to run on opening a PR (like the calibre image compression action we do) or on merge (like the generate chapters script that runs) to ensure tests are run.
  • Potentially some longer actions run periodically (nightly?)
  • Production monitoring (perhaps using the likes of UptimeRobot)?

Potentially Docker would help with some automated testing (through Github Actions or the like) rather than having to run the full dependency installs each time?

@ibnesayeed
Copy link
Contributor

  • Add HTML/JS/CSS linters.

https://github.blog/2020-06-18-introducing-github-super-linter-one-linter-to-rule-them-all/

@tunetheweb
Copy link
Member Author

  • Add HTML/JS/CSS linters.

https://github.blog/2020-06-18-introducing-github-super-linter-one-linter-to-rule-them-all/

I am loving this! Example output here: https://github.com/bazzadp/almanac.httparchive.org/runs/819255550?check_suite_focus=true

Need to do some cleanup before we enable it for some code types but looks great!

@ibnesayeed
Copy link
Contributor

I am loving this! Example output here: https://github.com/bazzadp/almanac.httparchive.org/runs/819255550?check_suite_focus=true

Need to do some cleanup before we enable it for some code types but looks great!

I would suggest adding any exceptions only when necessary. Though, this means there will be a really big PR to eliminate all the linting issues.

@tunetheweb
Copy link
Member Author

Is it possible to add exceptions without importing the whole TEMPLATE file and customising it? Or is that what you are supposed to do?

@ibnesayeed
Copy link
Contributor

ibnesayeed commented Jun 29, 2020

Is it possible to add exceptions without importing the whole TEMPLATE file and customising it? Or is that what you are supposed to do?

It is nothing but a curated collection of popular linters in various languages, so the options of disabling certain rules inline or via config are supposed to be the way those linters in different languages expect them to be. Read more about Disabling linters and Rules for various languages.

@ibnesayeed
Copy link
Contributor

I've not used Docker much to be honest. What would that give us?

  • A ready to roll image that will have all the necessary dependencies pre-installed and fully configured to the need of the app
  • Isolation from the host machine's environment, so nothing used for this app can affect other application or host and vice versa
  • Reliable cleanup of the system when done working on this project so no dependencies or artifacts are littered in the host machine
  • Detecting missing dependencies while developing as they will be caught in the encapsulated environment, but if a developer had something from another project, they might fail to realize they need to add the dependency explicitly
  • Running multiple instances with different changes and configurations at the same time
  • Keeping a versioned history of the system at important breakpoints
  • Consistent reproducibility across various machines
  • Running tests locally in a container and replicating the same in the CI platforms such as GH Workflows

But not sure if installing Docker would take just as long for those of us that don't use it, and would give us another thing to maintain?

Including a Dockerfile does not mean people must use it. Those who prefer the traditional route may continue to run the application directly on their host machine. Docker is a layer of indirection, so it solves some problems, but that also means this itself needs to be installed on the machine. However, it is a one time investment, so those who use Docker for every project start to see the advantage. I personally do not have any development environment setup on my machine, but be it a specific version of Python, Node, Ruby, Golang, or anything else I need to contribute to a project or to play around with something quick, I simply spin a relevant throwaway container, do the needful, and I am back to the clean slate as before.

@tunetheweb
Copy link
Member Author

OK lets gives it a go. You want to submit a PR with a Dockerfile @ibnesayeed ?

@ibnesayeed
Copy link
Contributor

OK lets gives it a go. You want to submit a PR with a Dockerfile @ibnesayeed ?

I will give it a go over the weekend hopefully.

@ibnesayeed
Copy link
Contributor

With #956 merged, one more of the checkboxes can be checked.

@rviscomi rviscomi added this to the 2020 Platform Development milestone Jul 6, 2020
@tunetheweb
Copy link
Member Author

GitHub Superlinter added HTMLHint HTML linter. Will have a play this weekend and see if we can use it. We use Jinja2 templates rather than pure HTML so suspect it may not like it but will give it a go.

Unfortunately there does not seem to be many Jinja2 linters about. The few that are there seem
old and poorly maintained. Or perhaps they are so simple they don’t need maintenance- but that suggests they are just looking at the Jinja2 code and not HTML and ideally we want both.

Won’t pull out into a separate issue until I’m more certain it’s likely achievable with linting the source files (the other alternative is to lint the output but that’ll be more complicated). In meantime I’m fine tracking it here until then. But if we close this issue I definitely will move it out.

This was referenced Jul 11, 2020
@ibnesayeed
Copy link
Contributor

@bazzadp I know you are a big fan of keeping dependencies to their latest versions. As a result you may end up specifying dependencies without a version number. However, this is a brittle approach, especially when a repository is not maintained actively and breaking changes pile up. On the other hand, version pinning of dependencies makes a system use stale and often insecure versions, because actively monitoring and keeping up with new releases of all the dependencies is difficult. How about we use something like GitHub Dependabot that can handle periodic check for dependencies of various languages, and more recently they even added support for dependency check of GitHub Actions. This requires adding a file under the .github directory with our desired configurations and will create PRs when a dependency is updated, so we can simply test it and merge.

@tunetheweb
Copy link
Member Author

Agreed. That way we keep things up to date but can also control updates. I believe the main HTTPArchive repo uses it already so had been meaning to look into this at some point. You want to take this?

Should we raise a separate issue for this if there are things to discuss or several PRs to identify? Or is it simple enough to cover in one PR so don’t need an issue on top of this?

@ibnesayeed
Copy link
Contributor

You want to take this?

My plate is too full right now and I have some super important deadlines to meet, so unfortunately I cannot promise to actively contribute in the coming weeks.

Should we raise a separate issue for this if there are things to discuss or several PRs to identify? Or is it simple enough to cover in one PR so don’t need an issue on top of this?

My assessment suggests that one PR should be enough to at leas get the bot configured and perform an initial version pinning of every dependency (Python, JavaScript, and Actions etec.). We will have to define the frequency for each language, any exceptions, and update policies. Things will roll from there and the bot will do its job to periodically creating PRs.

@tunetheweb
Copy link
Member Author

Actually I’ve just checked and it’s already enabled and was able to pick up our 3 version files (requirements.txt, package.json and package-lock.json) even without a config file.

However it seems to only open PRs for security issues not regular updates. So I still think good to lock to major version but allow automatic minor version upgrades.

@ibnesayeed
Copy link
Contributor

Actually I’ve just checked and it’s already enabled and was able to pick up our 3 version files (requirements.txt, package.json and package-lock.json) even without a config file.

Earlier it was a third-party service, so it was enabled by giving access to their app, but last year GitHub acquired it and earlier last month GitHub started supporting a new version of it natively. In the process a few things have changed and I think it is now configured more seamlessly without giving access to a third-party app. Now you can place a config file under .github folder and you are good to go (read the docs).

@ibnesayeed
Copy link
Contributor

Well, I did add a basic config file in RP #1004. @bazzadp you may want to add it in the checklist of this ticket and mark it done.

@tunetheweb
Copy link
Member Author

tunetheweb commented Jul 12, 2020

Well, I did add a basic config file in RP #1004. @bazzadp you may want to add it in the checklist of this ticket and mark it done.

Done. Making great progress in improving the text stack!

@tunetheweb
Copy link
Member Author

@HTTPArchive/developers anyone used the Lighthouse CLI before that could integrate this into our generate script and add some performance budgets?

@tunetheweb
Copy link
Member Author

I'm going to close this issue. Think we've made fantastic progress on improving testing this year! Much more confident in merge changes and releasing the site.

Two things I'd still like to continue on are are covered in separate issues:

  • Improving pytest Increase pytest coverage #1229
  • Automatic Lighthouse testing Add Lighthouse testing #1178
    Not convinced to the need for Cypress testing at the moment unless we make the site a lot more complicated and think the HTML linting we have is sufficient coverage of what a lot of that would covered.

Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Building the Almanac tech stack good first issue Good for newcomers question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants