jena icon indicating copy to clipboard operation
jena copied to clipboard

GH-1306: Upgrade Jena UI to Vue 3

Open kinow opened this issue 3 years ago • 7 comments

GitHub issue resolved #1306

Pull request Description:

Upgrade Jena UI to Vue 3


  • [x] Tests are included.
  • [ ] Documentation change and updates are provided for the Apache Jena website
  • [x] Commits have been squashed to remove intermediate development commit messages.
  • [x] Key commit messages start with the issue number (GH-xxxx or JENA-xxxx)

By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.


See the Apache Jena "Contributing" guide.

kinow avatar Aug 14 '22 21:08 kinow

Another PR related to Vue, but this time upgrading from 2.x to 3.x. Unfortunately this may take a little while, even though it has 99% done. Pending fixing the tests, and a blocker issue https://github.com/vuejs/router/issues/454 (that we may be able to work around, but takes a while to test).

I've also updated the pom.xml to run the e2e integration tests with Maven. @afs normally I use GitHub Actions with Cypress e2e tests, as it's easy to configure Node + Chrome. Is that something that we could enable in this repository for the UI, or do you prefer that it's enabled in ASF Jenkins?

Thanks

kinow avatar Aug 14 '22 21:08 kinow

And a list of things pending in this PR to track the progress:

  • [x] Upgraded a per Vue docs
  • [x] The project builds with no warnings
  • [x] UI starts
  • [x] UI unit & e2e tests pass
  • [x] Manual tests find no issues

kinow avatar Aug 14 '22 21:08 kinow

GitHub Actions

Go for it. We have some actions (e.g. mac testing).

The thing to watch is that all clones of the repo also get the actions.

Jena's current actions are manually run:

  1. automatic actions risks hitting the cloning GH user's account on cloned repos. Oops.
  2. it's a waste to run these additional tests automatically. We have ASF Jenkins all setup to trigger and these jobs don't get inherited. For a java project, running one on-push job and a few regular (e.g. weekly) jobs is more than enough.

afs avatar Aug 15 '22 07:08 afs

Go for it. We have some actions (e.g. mac testing).

Perfect, thanks @afs !

The thing to watch is that all clones of the repo also get the actions.

Jena's current actions are manually run:

automatic actions risks hitting the cloning GH user's account on cloned repos. Oops.

Maybe we could control where the jobs run. e.g.: https://github.com/orgs/community/discussions/26704#discussioncomment-3252979 & https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idif

# ...
name: jena-ui
on: [push, workflow_dispatch]
jobs:
  build-ui:
    if: github.repository == ‘apache/jena’
    steps:
      - uses: actions/checkout@v3
      - ...
      - ...
 
#...

Or users could disable the actions in their forks.

Or we can change the trigger event of GH Actions (see below).

it's a waste to run these additional tests automatically. We have ASF Jenkins all setup to trigger and these jobs don't get inherited. For a java project, running one on-push job and a few regular (e.g. weekly) jobs is more than enough.

That's a good point.

I think we can also control it. For example, the project I worked today only builds for code pushed to the main branch, or to pull requests created against the main branch: https://github.com/kinow/cwlviewer/blob/f1091b4ad96236be4eb78df8451e27a81fdb972a/.github/workflows/ci-build.yml#L3-L9

My fork has not run any GH Actions, even though I created several PR's against the main repository: https://github.com/kinow/cwlviewer/actions/

Because I never created a PR on my fork to the main branch of my repository. I haven't tried it but I believe if I did that, it would trigger a PR on my fork - which I agree would be a waste of time & resources.

The main repository contains the Actions triggered on: pull_request: https://github.com/common-workflow-language/cwlviewer/pull/426/checks

And since it was a pull_request event, it's checking the merge commit (IIUC what the code will look like after it's merged): https://github.com/actions/checkout/issues/15#issuecomment-524093065 (that can be controlled too)

image

I have seen the Jenkins jobs in the ASF Jenkins, but I will confess that I rarely verify the status there. I normally run things locally manually, but occasionally I forget and find that my branches are broken. Normally I find it before calling for a final review.

Maybe we could have Jenkins pushing notifications to GH pull requests in the future about the job status? :grimacing: That way I would fix my PR's sooner.

Let me know if you'd like this e2e GH actions to run only against main as in the cwlviewer case, or manually, and I'll update this PR. Thanks Andy!!!

kinow avatar Aug 15 '22 09:08 kinow

Found a workaround for the Vue Router blocker, that almost worked. I had to tweak it a little more to get everything working. Then spent some time working on warnings and minor issues with the YASG/YASQE and the file upload components.

All manual tests passed. Linter passed with no errors. Updated dependencies once again with ncu -u and yarn install. The unit tests passed after adjusting the testing libraries (that was updated too, along with Vue 3). Finally, fixing the e2e tests was easy, but good because these tests detected an important bug that would have passed by my tests.

It was when you had pagination in the table, like when you have more than 5 datasets. Pagination wasn't working due to a mistake on how to use v-model with Vue 3.

@afs when you have some spare time to review this one, I think it would be interesting for the review:

  • see if you spot anything that looks odd in the code;
  • confirm it works some different way than what I used (I used Eclipse + WebStorm, running FusekiCMD on the latter, and yarn run server on the former/terminal);
  • confirm there is nothing in the UI that looks odd (I cannot tell anymore, after looking at the UI for so long, but I will take another look in a few weeks);
  • test some crazy things, like a query that's more complex than the simple one I used, or edit a graph (I tested it, but who knows), or if you see a table with pagination, then force it to have at least more than 1 page, etc.

Once this is merged our dependencies should be using the latest available for Vue and related dependencies. When I started this rewrite my goal was a) replace Backbone by something else like Vue ✅, b) add dependency management ✅, c) add tests both unit and integration/e2e ✅, and then d) upgrade to Vue 3 ⏹️.

Other plans include improving the UI for accessibility (translation, color schemas, dark/light modes, vision-impaired users, etc), UIUX improvements, and allow for extensibility (like having plug-ins, or re-using parts of the UI in other applications). Almost there :beers:

Cheers -Bruno

kinow avatar Sep 20 '22 12:09 kinow

I pulled the PR branch - the head commit is 81a8b446f4.

Are you going to squash the WIP commits before merge?


I get some test failures, building from the top or just the module. This may well be because the toolchain is not up to date (how/what can I check?) but they are timeouts so could that be the issue?

Then, I pushed the PR branch to my Jena working copy and ran the github action for Linux:

The UI step worked (the build failed for other reasons - it does sometimes) https://github.com/afs/jena/actions/runs/3095974510/jobs/5010983697

cd jena-fuseki-ui
mvn clean install

gives (extract below - full output 2022-09-21-ui-install.txt):

[INFO] [TESTS] ================================================================================
[INFO] [TESTS] 
[INFO] [TESTS]   (Run Starting)
[INFO] [TESTS] 
[INFO] [TESTS]   ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
[INFO] [TESTS]   │ Cypress:        10.8.0                                                                         │
[INFO] [TESTS]   │ Browser:        Electron 102 (headless)                                                        │
[INFO] [TESTS]   │ Node Version:   v16.13.1 (/home/afs/Jena/jena-fuseki2/jena-fuseki-ui/node/node)                │
[INFO] [TESTS]   │ Specs:          2 found (datasets.cy.js, query.cy.js)                                          │
[INFO] [TESTS]   │ Searched:       tests/e2e/specs                                                                │
[INFO] [TESTS]   └────────────────────────────────────────────────────────────────────────────────────────────────┘
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS] ────────────────────────────────────────────────────────────────────────────────────────────────────
[INFO] [TESTS]                                                                                                     
[INFO] [TESTS]   Running:  datasets.cy.js                                                                  (1 of 2)
[INFO] [TESTS]  DONE  Compiled successfully in 1750ms08:58:47
[INFO] [TESTS] 
[INFO] [TESTS]  DONE  Compiled successfully in 2023ms08:58:47
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS]   datasets
[INFO] [TESTS]     1) Visits datasets page, also the application landing-page
[INFO] [TESTS]     2) Filters without any data
[INFO] [TESTS]     After creating new datasets
[INFO] [TESTS]       3) "before all" hook for "Visits datasets page"
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS]   0 passing (24s)
[INFO] [TESTS]   3 failing
[INFO] [TESTS] 
[INFO] [TESTS]   1) datasets
[INFO] [TESTS]        Visits datasets page, also the application landing-page:
[INFO] [TESTS]      AssertionError: Timed out retrying after 7500ms: Expected to find element: `h2.text-center`, but never found it.
[INFO] [TESTS]       at Context.eval (webpack://apache-jena-fuseki/./tests/e2e/specs/datasets.cy.js:31:7)
[INFO] [TESTS] 
[INFO] [TESTS]   2) datasets
[INFO] [TESTS]        Filters without any data:
[INFO] [TESTS]      AssertionError: Timed out retrying after 7500ms: Expected to find element: `#filterInput`, but never found it.
[INFO] [TESTS]       at Context.eval (webpack://apache-jena-fuseki/./tests/e2e/specs/datasets.cy.js:43:7)
[INFO] [TESTS] 
[INFO] [TESTS]   3) datasets
[INFO] [TESTS]        After creating new datasets
[INFO] [TESTS]          "before all" hook for "Visits datasets page":
[INFO] [TESTS]      AssertionError: Timed out retrying after 7500ms: Expected to find element: `#dataset-name`, but never found it.
[INFO] [TESTS] 
[INFO] [TESTS] Because this error occurred during a `before all` hook we are skipping the remaining tests in the current suite: `After creating new datasets`
[INFO] [TESTS]       at Context.eval (webpack://apache-jena-fuseki/./tests/e2e/specs/datasets.cy.js:60:15)
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS]   (Results)
[INFO] [TESTS] 
[INFO] [TESTS]   ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
[INFO] [TESTS]   │ Tests:        9                                                                                │
[INFO] [TESTS]   │ Passing:      0                                                                                │
[INFO] [TESTS]   │ Failing:      3                                                                                │
[INFO] [TESTS]   │ Pending:      0                                                                                │
[INFO] [TESTS]   │ Skipped:      6                                                                                │
[INFO] [TESTS]   │ Screenshots:  3                                                                                │
[INFO] [TESTS]   │ Video:        false                                                                            │
[INFO] [TESTS]   │ Duration:     23 seconds                                                                       │
[INFO] [TESTS]   │ Spec Ran:     datasets.cy.js                                                                   │
[INFO] [TESTS]   └────────────────────────────────────────────────────────────────────────────────────────────────┘
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS]   (Screenshots)
[INFO] [TESTS] 
[INFO] [TESTS]   -  tests/e2e/screenshots/datasets.cy.js/datasets -- Visits datasets page, also the      (1280x720)
[INFO] [TESTS]      application landing-page (failed).png                                                          
[INFO] [TESTS]   -  tests/e2e/screenshots/datasets.cy.js/datasets -- Filters without any data (faile     (1280x720)
[INFO] [TESTS]      d).png                                                                                         
[INFO] [TESTS]   -  tests/e2e/screenshots/datasets.cy.js/datasets -- After creating new datasets --      (1280x720)
[INFO] [TESTS]      Visits datasets page -- before all hook (failed).png                                           
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS] ────────────────────────────────────────────────────────────────────────────────────────────────────
[INFO] [TESTS]                                                                                                     
[INFO] [TESTS]   Running:  query.cy.js                                                                     (2 of 2)
[INFO] [TESTS]  DONE  Compiled successfully in 415ms08:59:13
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS]   Query
[INFO] [TESTS]     1) "before all" hook for "Uses the correct SPARQL Endpoint"
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS]   0 passing (8s)
[INFO] [TESTS]   1 failing
[INFO] [TESTS] 
[INFO] [TESTS]   1) Query
[INFO] [TESTS]        "before all" hook for "Uses the correct SPARQL Endpoint":
[INFO] [TESTS]      AssertionError: Timed out retrying after 7500ms: Expected to find element: `#dataset-name`, but never found it.
[INFO] [TESTS] 
[INFO] [TESTS] Because this error occurred during a `before all` hook we are skipping the remaining tests in the current suite: `Query`
[INFO] [TESTS]       at Context.eval (webpack://apache-jena-fuseki/./tests/e2e/specs/query.cy.js:30:11)
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS]   (Results)
[INFO] [TESTS] 
[INFO] [TESTS]   ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
[INFO] [TESTS]   │ Tests:        1                                                                                │
[INFO] [TESTS]   │ Passing:      0                                                                                │
[INFO] [TESTS]   │ Failing:      1                                                                                │
[INFO] [TESTS]   │ Pending:      0                                                                                │
[INFO] [TESTS]   │ Skipped:      0                                                                                │
[INFO] [TESTS]   │ Screenshots:  1                                                                                │
[INFO] [TESTS]   │ Video:        false                                                                            │
[INFO] [TESTS]   │ Duration:     7 seconds                                                                        │
[INFO] [TESTS]   │ Spec Ran:     query.cy.js                                                                      │
[INFO] [TESTS]   └────────────────────────────────────────────────────────────────────────────────────────────────┘
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS]   (Screenshots)
[INFO] [TESTS] 
[INFO] [TESTS]   -  tests/e2e/screenshots/query.cy.js/Query -- Uses the correct SPARQL Endpoint -- b     (1280x720)
[INFO] [TESTS]      efore all hook (failed).png                                                                    
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS] ================================================================================
[INFO] [TESTS] 
[INFO] [TESTS]   (Run Finished)
[INFO] [TESTS] 
[INFO] [TESTS] 
[INFO] [TESTS]        Spec                                              Tests  Passing  Failing  Pending  Skipped  
[INFO] [TESTS]   ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
[INFO] [TESTS]   │ ✖  datasets.cy.js                           00:23        9        -        3        -        6 │
[INFO] [TESTS]   ├────────────────────────────────────────────────────────────────────────────────────────────────┤
[INFO] [TESTS]   │ ✖  query.cy.js                              00:07        1        -        1        -        - │
[INFO] [TESTS]   └────────────────────────────────────────────────────────────────────────────────────────────────┘
[INFO] [TESTS]     ✖  2 of 2 failed (100%)                     00:31       10        -        4        -        6  
[INFO] [TESTS] 
[INFO] [TESTS]  ERROR  Error: Command failed: /home/afs/Jena/jena-fuseki2/jena-fuseki-ui/node_modules/cypress/bin/cypress run --config baseUrl=http://XPS-afs-9310:8080 --headless
[INFO] [TESTS] Error: Command failed: /home/afs/Jena/jena-fuseki2/jena-fuseki-ui/node_modules/cypress/bin/cypress run --config baseUrl=http://XPS-afs-9310:8080 --headless
[INFO] [TESTS]     at makeError (/home/afs/Jena/jena-fuseki2/jena-fuseki-ui/node_modules/execa/index.js:174:9)
[INFO] [TESTS]     at /home/afs/Jena/jena-fuseki2/jena-fuseki-ui/node_modules/execa/index.js:278:16
[INFO] [TESTS]     at processTicksAndRejections (node:internal/process/task_queues:96:5)
[INFO] [TESTS] vue-cli-service test:e2e --headless exited with code 1

afs avatar Sep 21 '22 08:09 afs

HI @afs

I pulled the PR branch - the head commit is 81a8b44.

Are you going to squash the WIP commits before merge?

Just did that :+1: Left two commits since one of them is actually enabling the e2e tests to run in Maven (only unit tests were part of the Maven build before).

I get some test failures, building from the top or just the module. This may well be because the toolchain is not up to date (how/what can I check?) but they are timeouts so could that be the issue?

Could be. Cypress is not a small executable, nor just a few JS files. Last I checked it brought several MB to run it, and is memory hungry too. @afs if you prefer I can remove it from the Maven build, and we can run it manually in our local workstations and on Jenkins or GH Actions.

Then, I pushed the PR branch to my Jena working copy and ran the github action for Linux:

The UI step worked (the build failed for other reasons - it does sometimes) https://github.com/afs/jena/actions/runs/3095974510/jobs/5010983697

:+1: It works for me locally too, but it could be because I have everything installed globally (Cypress/Yarn/etc) as I use it for other projects too, or because I used it manually to run the e2e tests so it may have downloaded something else.

kinow@ranma:~/Development/java/jena/jena$ mvn clean test install -Pdev
# ...
[INFO] Reactor Summary for Apache Jena 4.7.0-SNAPSHOT:
[INFO] 
[INFO] Apache Jena ........................................ SUCCESS [ 12.422 s]
[INFO] Apache Jena - Shadowed external libraries .......... SUCCESS [  3.357 s]
[INFO] Apache Jena - IRI .................................. SUCCESS [  5.164 s]
[INFO] Apache Jena - Base ................................. SUCCESS [  8.698 s]
[INFO] Apache Jena - Core ................................. SUCCESS [01:07 min]
[INFO] Apache Jena - ARQ .................................. SUCCESS [01:50 min]
[INFO] Apache Jena - SHACL ................................ SUCCESS [ 11.209 s]
[INFO] Apache Jena - ShEx ................................. SUCCESS [  9.122 s]
[INFO] Apache Jena - RDF Connection ....................... SUCCESS [  4.266 s]
[INFO] Apache Jena - DBOE Database Operation Environment .. SUCCESS [  0.080 s]
[INFO] Apache Jena - DBOE Base ............................ SUCCESS [  3.119 s]
[INFO] Apache Jena - DBOE Transactions .................... SUCCESS [  2.915 s]
[INFO] Apache Jena - DBOE Indexes ......................... SUCCESS [  0.707 s]
[INFO] Apache Jena - DBOE Index test suite ................ SUCCESS [  0.868 s]
[INFO] Apache Jena - DBOE Transactional Datastructures .... SUCCESS [  7.366 s]
[INFO] Apache Jena - DBOE Storage ......................... SUCCESS [  3.164 s]
[INFO] Apache Jena - TDB1 (Native Triple Store) ........... SUCCESS [ 21.734 s]
[INFO] Apache Jena - TDB2 (Native Triple Store) ........... SUCCESS [ 33.291 s]
[INFO] Apache Jena - Libraries POM ........................ SUCCESS [  0.538 s]
[INFO] Apache Jena - Command line tools ................... SUCCESS [ 10.126 s]
[INFO] Apache Jena - SPARQL Text Search ................... SUCCESS [ 17.249 s]
[INFO] Apache Jena - GeoSPARQL Engine ..................... SUCCESS [ 18.874 s]
[INFO] Apache Jena - Fuseki - A SPARQL 1.1 Server ......... SUCCESS [  0.035 s]
[INFO] Apache Jena - Fuseki Core Engine ................... SUCCESS [  4.532 s]
[INFO] Apache Jena - Fuseki UI ............................ SUCCESS [03:48 min]
[INFO] Apache Jena - Fuseki Data Access Control ........... SUCCESS [  4.916 s]
[INFO] Apache Jena - Fuseki Server Main ................... SUCCESS [ 11.899 s]
[INFO] Apache Jena - Fuseki Server Jar .................... SUCCESS [  4.137 s]
[INFO] Apache Jena - Fuseki Webapp ........................ SUCCESS [ 12.688 s]
[INFO] Apache Jena - Fuseki WAR File ...................... SUCCESS [  2.569 s]
[INFO] Apache Jena - Fuseki Server Standalone Jar ......... SUCCESS [  4.718 s]
[INFO] Apache Jena - Fuseki Docker Tools .................. SUCCESS [  0.332 s]
[INFO] Apache Jena - Fuseki Binary Distribution ........... SUCCESS [  3.981 s]
[INFO] Apache Jena - Integration Testing .................. SUCCESS [ 39.515 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  11:11 min
[INFO] Finished at: 2022-09-22T09:17:56+12:00
[INFO] ------------------------------------------------------------------------


kinow@ranma:~/Development/java/jena/jena$ git log -n 2
commit e3719e7299d3f0212664f4ba6ac383b09e767443 (HEAD -> vue3, kinow/vue3)
Author: Bruno P. Kinoshita <[email protected]>
Date:   Tue Jul 26 14:19:16 2022 +1200

    GH-1306: Upgrade Jena UI to Vue3

commit f8cce7cefbd7344f056798a7e795ee3c370569cc
Author: Bruno P. Kinoshita <[email protected]>
Date:   Mon Aug 15 09:08:54 2022 +1200

    GH-1306: Enable e2e tests in CI

kinow avatar Sep 21 '22 21:09 kinow

Some news: I've managed to get a successful run of the PR.

I had to ensure HOST is unset or set to localhost.

(there might be an issue with cypress needing to be installed manually on a clean machine but I can't confirm)

afs avatar Oct 29 '22 17:10 afs

Looks good!

Only found a couple of small things:

1: .nyc_output/out.json gets created. .nyc_output/** needs to go in the .gitignore in jena-fuseki-ui

2: The logo has disappeared: Old: Screenshot from 2022-10-29 18-50-01 New: Screenshot from 2022-10-29 18-50-49

afs avatar Oct 29 '22 17:10 afs

These are easy to fix. I will rebase and fix these issues in the next few days. Thanks @afs!

kinow avatar Oct 30 '22 01:10 kinow

2: The logo has disappeared:

Strange. Even though we had img width=52px, that value was being reset to 0. So the image was there, but not displayed. Used style="width: 52px" and that solved it :man_shrugging:

1: .nyc_output/out.json gets created. .nyc_output/** needs to go in the .gitignore in jena-fuseki-ui

Added!

Thanks @afs !

kinow avatar Nov 01 '22 15:11 kinow

Yes - that is weird. Something (tm) put in that width=0.

afs avatar Nov 01 '22 16:11 afs

Ah, we can leave the style=, but I found the culprit: https://stackoverflow.com/questions/64108443/image-height-is-reset-to-0-after-upgrading-to-vue-3

Either way the result for users is the same. No strong preference on any approach here as it's for a single image.

kinow avatar Nov 01 '22 16:11 kinow