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

Auto-test as default, option to switch off? #76

Closed
katrinleinweber opened this issue Mar 8, 2018 · 15 comments
Closed

Auto-test as default, option to switch off? #76

katrinleinweber opened this issue Mar 8, 2018 · 15 comments

Comments

@katrinleinweber
Copy link

Hello! As a 1st-time user here, but a few-months-long excercist (SCNR) ;-) I'm wondering whether translator development could be improved (faster feedback) by Scaffold bein upgraded to auto-run:

a) a test that was changed, and/or
b) all tests if some code was changed?

Presumably there are edge cases in which an automation would not be desirable, so those might need a changed behaviour of what is currently the Run button (for example) to be toggle-able to Stop.

What do you think about this idea?

@adam3smith
Copy link
Collaborator

We have some pretty massive tests -- what exactly would trigger this for b)? It can't be every code change. Save also doesn't work because you'll want to allow frequent saving e.g for browser testing and not wait to write the save operation while tests are running.

@katrinleinweber
Copy link
Author

"massive tests" in a single translator or a massive test suite over all? Are we talking about seconds or minutes?

I believe that most other testing tools I've heard of, trigger after saving the file, so there would be less chance of a queue. Assuming the tests are indeed faster than a user would need to conduct the next modification.

Maybe I just got lucky with Zenodo.js's tests: took a few seconds only.

@adam3smith
Copy link
Collaborator

We have test suites with up to 20 tests for a single translator and depending on the connection they can take minutes, especially when they use defer.
The typical software test scenario are CI tests run server-side, though, no? I'm not aware of anything that automatically runs local tests? Nothing else in the Zotero universe does.

@katrinleinweber
Copy link
Author

Interesting! I got introduced to automatic tests through exercisms R track, and tested locally using testthat. Same with the Python tutorial(s) in PyCharm Edu. With feedback cycles of just a few seconds, it became an exciting breeze to exercise through those coding tutorials :-) That breeze was not there when I worked with Scaffold, because switching tabs and pressing buttons to manually trigger the test runs took almost as long, as they actually ran :-(

Back to the topic, sorry: If defer is an indicator of slow(er) tests, that could be used as an automatic exception to a default auto-testing upgrade.

@adam3smith
Copy link
Collaborator

Hmmm -- I guess I don't see the scenario where you'd want to constantly run tests. I wouldn't typically run tests frequently when developing nor do I think of it as good practice. I've always viewed the main function of the tests to allow for automated monitoring of translators: https://zotero-translator-tests.s3.amazonaws.com/index.html (that's unfortunately not working as well as it used to when Zotero ran entirely in Firefox: the tests suggests way more translators are broken than is actually the case)

To check on your translator code, it's typically a better idea to actually try it out with doWeb and inspect the output rather than a diff. DoWeb does tell you if you've actually broken something, so basic functionality is covered with that. Other than typical software tests, you also don't necessarily need to pass translator tests after making changes. I'd say about 80% of the time, I update the tests after making translator changes.

The test comes in handy at the end to check if you've introduced any regressions to the imported data quality with a change and as a good way to review translators quickly for us, but I don't see a ton of use in suggesting to people that they're more canonical than they really are.

defer is one indicator of tests that'll take longer (it introduces a 30(?) second wait in the tests), but another reason for slow tests may just be a slow internet connection and/or a slow server. All tests except those for import translators make http queries -- they're going to be much slower than tests against local data. I'm also not sure that comparing tests in tutorials to tests on production code is fair ;) Tutorials have right answers...

@katrinleinweber
Copy link
Author

Well, production code that passes the tests, is one right answer ;-) I think we disagree about how quickly the feedback whether a code or test change still along a right track should be given.

@adam3smith
Copy link
Collaborator

I don't think we disagree about the importance of rapid feedback -- I think e.g. live code check as recently implemented in Scaffold is great. I think the fact that Scaffold allows you to one-click-test import makes it indispendable. I disagree that translator tests help with that because
a) As a general rule they're not rapid and
b) They provide a baseline, often based on a previous version of a site.

I'm actually a bit confused at what point during the changes you made to Zenodo you would have found more testing useful? Literally every change you made changed the tests.

@katrinleinweber
Copy link
Author

Not more testing per se, just without having to switch to a dedicated "Testng" tab, select a/the test(s) and click a button manually.

@zuphilip
Copy link
Collaborator

I agree here with @adam3smith. I usually have one website loaded in the browser tab which I can during development use with the detectWeb and doWeb buttons.

just without having to switch to a dedicated "Testing" tab, select a/the test(s) and click a button manually.

Maybe a short-cut could be enough to save the several clicks or another button which is doing that? At least this does not sound that you need the tests to run automatically. Running tests automatically sounds like a huge work and something were a lot of things can probably break.

One thing which I recently thought about being handy, would be to copy and open a url from the test tab to the browsing tab, i.e. "Load in Browser/Browsing Tab". Would this be helpful?

@katrinleinweber
Copy link
Author

Not sure. I seem to have worked differently. I used the Browser tab only to open a URL copied from the actual browser, in order to then use Testing > New Web and Run to check the output. Temporary test so to speak.

Yes, setting up automated testing is a big task in itself, but it improves the coding experience tremendously. One could interpret the fact that many projects include very easy-to-follow test instructions in their CONTRIBUTING.md (single or very few lines of copy-paste-able code, which often triggers an automatic test runner) as a strong motivator for contributions to a project.

@dstillman
Copy link
Member

Yeah, I don't think running tests automatically makes sense — there's no need to hammer sites with requests when you as the author know when you actually have something to test.

As @adam3smith and @zuphilip say, translator tests aren't really useful during development, because you don't have the data yet. I could see a small bit of value in running a specific test from the code tab when repairing a broken site, but I'm not sure it's worth the UI confusion (i.e., the notion of a test still being selected in a non-visible pane). There could perhaps be a menubutton next to doWeb() with a drop-down of the different URLs from the tests, but I'm not convinced it's necessary — when sites break, it often results in translator errors, not incorrect data, so just loading the URL and checking the doWeb() output is usually pretty sufficient.

One thing which I recently thought about being handy, would be to copy and open a url from the test tab to the browsing tab, i.e. "Load in Browser/Browsing Tab". Would this be helpful?

I do think this would be useful.

@katrinleinweber
Copy link
Author

[…] there's no need to hammer sites with requests when you as the author know when you actually have something to test.

That's what memoization prevents. Also in JavaScript.

[…] aren't really useful during development, because you don't have the data yet.

In TDD, you'd specify in the test (or a small portion of it), which data you want, then write the code that scrapes it so.

I could see a small bit of value in running a specific test from the code tab when repairing a broken site […]

Or upgrading a translator.

but I'm not sure it's worth the UI confusion (i.e., the notion of a test still being selected in a non-visible pane).

The automation would of course need to select them. It already can populate the test list from the JSON, so an upgrade should be possible to make it detect which test (JSON block) changed and auto-run that test. Yes, in the background but of course with visual feedback in the right panel. What's it called? The one where the Test RegEx also prints ==>true<==. Test Frame?

There could perhaps be a menubutton next to doWeb() with a drop-down of the different URLs from the tests, but I'm not convinced it's necessary

Agreed, no buttons please! Except for one 1-click element to toggle auto-testing on and off.

when sites break, it often results in translator errors, not incorrect data, so just loading the URL and checking the doWeb() output is usually pretty sufficient.

Sufficient yes, but as I tried to express, it's just a different, better feeling to get test results within seconds of making a change, without any interaction except saving the file you were working on.

Really, if you haven't experienced it, I implore you to try the PyCharm or Kotlin Edu editions or exercism for a few hours.

But to make the point of this issue clear: Scaffold is already a big help :-) I wouldn't have touched translator development without it. The automation I'm suggesting is not essential, but would IMHO improve the convenience a lot.

@dstillman
Copy link
Member

In TDD, you'd specify in the test (or a small portion of it), which data you want, then write the code that scrapes it so.

We know what TDD is. We're saying it's a waste of time in the context of translator development, where the data already exists in the page or in a linked file and it's just a question of writing the right query selector or code to access it, at which point it becomes visible in detectWeb/doWeb output and provides the same immediate feedback you're describing. The test data can then be generated automatically at the end. There's no reason to be composing a JSON data structure by hand ahead of time.

I think we should have a button to open a test URL in the browser and keyboard shortcuts for detectWeb() and doWeb() in order to speed up the feedback cycle. If those don't exist, someone can open tickets for them. Other than that, I don't see us doing what you're asking for in this thread, so I'm going to close this. Thanks for the feedback.

@katrinleinweber
Copy link
Author

What I was asking for was a way to get that feedback without clicking around.

[…] just a question of writing the right query selector or code to access it

That may be the kind of code that an expert is probably able to "just" type down, but a beginner would be motivated to keep puzzling it together with better tooling.

@dstillman
Copy link
Member

What I was asking for was a way to get that feedback without clicking around.

Which is why I say there should be keyboard shortcuts for detectWeb/doWeb if there aren't already — you should be able to press Cmd-R to run doWeb(). But as we've explained, the translation process is too heavy to just run automatically whenever you stop typing.

That may be the kind of code that an expert is probably able to "just" type down, but a beginner would be motivated to keep puzzling it together with better tooling.

It really doesn't have anything to do with being an expert. All we're saying is that the output of detectWeb()/doWeb() provides the same feedback already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants