react is outdated

[yannickwurm]

We should be using something much more recent

[yannickwurm]

• React.createClass is gone => use ES6 classes (a few React components are already written this way)
• React.render is now ReactDOM.render. ReactDOM needs to be imported separately. First migrate to earliest react that had reactdom.render. Then check everything is working. Then increase react version numbers.
• Update React to latest
• Replace JSPM with NPM (but keep SystemJS which is stable - do not switch to webpack (unless person doing it can guarantee smooth transition)).
• We have rspec/ruby/capybara integration tests. It would be good to have unit tests for each ES6 class.

[yannickwurm]

• There is a different issue about potentially modularising code (i.e. using advanced or pragmatic react patterns). Such refactoring is independent of updating our basic tooling (i.e. the point listed above). It is also lower priority.
  (E.g., as part of a modularisation effort, one could model showErrorModal as Function-as-Component

[yannickwurm]

Hiya,
so @Lazy-poet has been doing this migration work. He's made huge progress. We also concluded that unit tests are needed for our JS things, and that RTL [React Testing Library] is the right approach for this. Now in trying to replace JSPM he hit a hurdle with the ruby integration. I'm copying from our other message thread.

First, I attempted to include jest and RTL into the project, but apparently, jest cannot resolve modules installed by jspm, so it was impossible to use. I then went ahead to convert all module installations to use npm, but then, trying to make SystemJS work with npm was another hassle, which leaves me with the option of migrating SystemJS to webpack. It was quite challenging, but I was able to set it up completely. another challenge was, some of the modules installed with jspm aren’t available on the nom registry, so I had to manually include them (2 of them).
Now, I have completely set it up, wrote a first test which passes, and the ui also works quite fine
Concerning the capybara_spec test, I'll like to know how '$" is meant to be included, because I get the error '$' is not defined when running the tests, so I'm presuming it's no longer loading jquery correctly. If I understand how jquery is being injected to it, then I'll know where to look to fix it.

Screenshots of successful JS test and failing ruby/rspec test:

@yeban or @ikk0v could you help to overcome this issue?

I think code is at https://github.com/Lazy-poet/sequenceserver or webpack/npm branch

[yannickwurm]

(brief followup coming from @Lazy-poet as some of what I wrote is outdated)

[Lazy-poet]

Hi all, so after making some changes (I had to manually declare jquery's $ as a global variable which wasn't required while running it locally on my machine), the initial error has disappeared, and now I'm left with 2 types of error, as shown in the images below

[ikozakov]

@Lazy-poet those errors from git@github.com:Lazy-poet/sequenceserver.git fork right? (not from the main repo?)

[Lazy-poet]

@ikk0v Yes from the fork, on the master branch

[ikozakov]

do you guys have any ideas on how to fix this? (maybe I'm missing some installation steps?)

(i get this when trying to build the app from the fork locally)

[Lazy-poet]

@ikk0v Oh yes, I just confirmed that the view files try to use the js files loaded by SystemJS in development mode, but SystemJS isn't in use no more. I have now modified the view files to use the build version in both development and production mode. You can pull again to see

[ikozakov]

This error comes from here:

We have 2 databases with the same name. Capybara#check method doesn't know which one to select. I'm not sure what changed here, but that's where the issue came from.

[ikozakov]

hmm, I cannot easily compare the main and fork repo, but, maybe you have any ideas why the fork would display 2 DB with the name "funky ids (v5)"?

Here is how it looks on the main repo:

I run the same scenario:
rspec -e "can download FASTA even if the hit ids are funky"

But you can see that the list of DBs is displayed differently if to compare the main and the fork.

[Lazy-poet]

over here, these are the dbs I can see, running on same forked repo

[yannickwurm]

Hmmm @ikozakov - there may be an issue with the css cache leading to a mix of old and new components. Does it hold if you clear cache / do private browsing in different browser?

[ikozakov]

over here, these are the dbs I can see, running on same forked repo

@Lazy-poet what scenario are you running?
is it this one rspec -e "can download FASTA even if the hit ids are funky"?

[ikozakov]

Does it hold if you clear cache / do private browsing in different browser?

capybara/selenium uses a clean browser for each test run, so it shouldn't be the problem, but let me check

[Lazy-poet]

the test passes when I run that-

[ikozakov]

the test passes when I run that-

ok, then back to your error above, which test failed when you got this:

[Lazy-poet]

@ikozakov also, you could check out the logs of the last travis build run. seems most of those initial test failures went away after my last push.

https://app.travis-ci.com/github/wurmlab/sequenceserver/builds/250126150

[ikozakov]

@ikozakov also, you could check out the logs of the last travis build run. seems most of those initial test failures went away after my last push.

i see now, awesome

[Lazy-poet]

the test passes when I run that-

ok, then back to your error above, which test failed when you got this:

this happened on "disables sequence viewer links if hits are longer than 10kb", but it no longer happens

[ikozakov]

@Lazy-poet do you have an option to re-run those tests on travis? looks like tests are not stable and fail from time to time.
(we have 3 failures now, but locally on my mac I see absolutely different picture)

Trigger build is disabled for me:

[Lazy-poet]

the build is disabled for me as well- but I can make an empty commit to force it to rerun

[yannickwurm]

(you both now should have the ability to click to rerun GitHub actions here by clicking)

i.e., on a PR, it would be here:

[Lazy-poet]

@ikozakov after rerunning the build, it's left with only one failing check- "a browser can show hit sequence in a modal"

[Lazy-poet]

I think I might have found the cause. currently investigating, and I'll revert in a few

[yeban]

@ikozakov wrote:

looks like tests are not stable and fail from time to time.

That's correct. I do not fully understand the cause of this.

I ran capybara_spec.rb yesterday on my Mac against the master branch and I got a failing test as well. For me the failing test was "a browser can run a simple tblastx search". The error message is similar to what was reported above:

Failure/Error: find('.dropdown-toggle').click

     Capybara::Ambiguous:
       Ambiguous match, found 2 elements matching visible css ".dropdown-toggle"

[Lazy-poet]

I think I might have found the cause. currently investigating, and I'll revert in a few