Add auto-detected home warning message mechanism

[edan-bainglass]

Based on #182

This PR adds a mechanism to sense if a home_app_warning.md file has been placed in the container's /home/<user>/.aiidalab directory, read it if so, and display it between the controls and the apps section.

Closes #184

[danielhollas]

This PR adds a mechanism to sense if a home_app_warning.md file has been placed in the container's /home//.aiidalab directory, read it if so, and display it between the controls and the apps section.

Can you explain a bit what is the use case of this? (ideally in an Github issue if it doesn't exist already)

[edan-bainglass]

Can you explain a bit what is the use case of this? (ideally in an Github issue if it doesn't exist already)

GitHub issue was attached to aiidalab-qe repo, now migrated here and mentioned in description 🙂

[danielhollas]

Code looks great, thank you for the cleanup as well. I do think this needs a test. I have just recently re-enabled selenium tests in this repo, using the same fixtures as in AWB, ao hopefully shouldn't be too much work.

[danielhollas]

This needs to be updated with changes on main.

I do think this needs a test. I have just recently re-enabled selenium tests in this repo, using the same fixtures as in AWB, ao hopefully shouldn't be too much work.

Let's not block this PR on tests in case writing one turns out to be more work, we can do it in a follow-up. Once this is merged I'll push a new aiidalab-home version.

[edan-bainglass]

@danielhollas FYI, in and out of vacation this week. But planning to wrap this up by end of week regardless.

[edan-bainglass]

@danielhollas can you take a look at my selenium test? Not sure why it can't find the element.

[danielhollas]

@edan-bainglass when debugging these tests, the first thing is to look at the screenshots that are uploaded as artifacts on GHA, e.g. here

Looking at this particular test, I can see there is no warning rendered

LMK in case I can be more helpful (I don't see anything obviously wrong in the test)

[edan-bainglass]

@edan-bainglass when debugging these tests, the first thing is to look at the screenshots that are uploaded as artifacts on GHA, e.g. here

Looking at this particular test, I can see there is no warning rendered

LMK in case I can be more helpful (I don't see anything obviously wrong in the test)

Right, I always forget about the screenshot. But yes, the issue is why is it not rendering the warning widget? I added an assert after creating the markdown file to ensure it is indeed there, which passes. So the file is there and the mechanism to render the file is there. Works locally. Not sure why it isn't passing.

I don't have much experience with selenium tests. It is unclear to me why something that works locally does not work remotely. Is there some refresh I need to trigger? Docs aren't clear. I think we should have our own docs for writing selenium, considering that it is often required for PRs.

[danielhollas]

I added an assert after creating the markdown file to ensure it is indeed there, which passes. So the file is there and the mechanism to render the file is there

Oh, that got me thinking, I don't think you're placing the file in the correct place. I suspect if you try to run the test locally, you'll see the file created in your local folder, but it needs to be created inside the container.

I think you need to use the aiidalab_exec fixture to create the file inside the container. Something like this

@pytest.fixture
def create_warning_file(aiidalab_exec):
      aiidalab_exec("echo 'This is a test warning' > /home/jovyan/.aiidalab/home_app_warning.md")

[edan-bainglass]

I added an assert after creating the markdown file to ensure it is indeed there, which passes. So the file is there and the mechanism to render the file is there

Oh, that got me thinking, I don't think you're placing the file in the correct place. I suspect if you try to run the test locally, you'll see the file created in your local folder, but it needs to be created inside the container.

I think you need to use the aiidalab_exec fixture to create the file inside the container. Something like this

@pytest.fixture
def create_warning_file(aiidalab_exec):
      aiidalab_exec("echo 'This is a test warning' > /home/jovyan/.aiidalab/home_app_warning.md")

Still raising an issue. I think this is closer to what we should do, but I guess I'm still not doing it correctly. If you understand how our fixtures work, can you please resolve this matter? And can we please document how to use our testing fixtures? Especially when it comes to selenium?

Update

The issue appears to be that I can't write to a nonexistent directory. Not sure why /home/jovyan/.aiidalab is nonexistent in an AiiDAlab container. But even if I add a mkdir -p /home/jovyan/.aiidalab command first, I still get that it is nonexistent. I think I'm gonna stop messing with this. We really need some documentation here. Also, it would be great to provide a simpler path for running a given selenium test locally. It is not ideal to have to go through the CI.

I'm happy to work on docs and local execution of selenium tests once I understand this whole thing better.

[danielhollas]

@edan-bainglass is it okay if I push to your branch? I can try something.

I can hear your frustration, and these tests are a pain. I do agree we need to document things better. One thing I'd like to do is to move from Selenium to Playwright (aiidalab/aiidalab-widgets-base#524) which should work better, has better docs and should have much better support for local testing. But in this case you're running more into issues with our Docker setup than anything else.

Did you try to run the tests locally? I believe if you have docker-compose installed it should work. It might need some extra installation for browser support. @yakutovicha I believe you were able to run selenium tests locally, would you mind to chime in here?

[edan-bainglass]

@danielhollas go for it 👍

[danielhollas]

The tests are passing! 🎉

The issue was annoying 😧 Basically, in aiidalab_exec fixture we were passing the command directly to docker-compose, but because we were not escaping the command in any way, it would not work for more complex commands that include for example redirection or chaining commands with &&. The solution to this is to wrap the command in bash -c 'command'.

You can easily see to problem by running the following locally

❯ docker run aiidalab/full-stack ls && cd apps/
apps
work
bash: cd: apps/: No such file or directory

versus

❯ docker run aiidalab/full-stack bash -c 'ls && cd apps/'
apps
work

@edan-bainglass have a look. All good from my side.

[edan-bainglass]

@danielhollas wow! What a struggle. Glad you were able to figure it out. Maybe we should have a chat soon to discuss a solid strategy for AiiDAlab app testing, even if it just means solid docs.

Anyhow, thanks for the work. Looks good!