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

feat(translation): using multilanguage features with Serenity/JS #1135

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

viper3400
Copy link
Sponsor Collaborator

This PR is just meant to start a discussion about this topic.

My main use case is having an application that supports multiple user languages. While the use cases stay the same, we have to locate page elements by a text in a different language or ensure, that the text of a page element is displayed in the right language.

Until now I have a translateTo function which takes an array of objects like

Const customTranslations = {
  SAVE: {
	En_Us: “Save”,
	De_Ch: “Speichern”,
	Fr_Ch: “Enregistrer”
   }
}

(See Gitter discussions, https://gitter.im/serenity-js/Lobby?at=611662b9f31bc0605a5e25b0)

I have a global configuration file, where the global locale for all test runs is set. But the requirement is to be able to set another locale for a certain test spec only, as well. (At the moment, this is just a simple reset of the locale from the global config object within the spec.)

Normally I’d put my Tasks and PageElements into classes. I try to separate my tests:

  • In my specs I just use Tasks that are imported from my Task classes. I try not to import any PageElements into the spec files directly.
  • In my Task classes I import from the PageElement classes.

Having the requirement to be able to change the locale from a certain test spec (and for this test spec only) I have to “inject” the locale into the Task class constructors and then into the PageElements class constructors as well and init the translateTo function with the right locale (and set of translation objects).

Hence, I cannot really use static PageElements or constants as this translate object has to be initialised each time before I can use it in a locator strategy like .where(Text, t(‘SAVE’)) for example.

So every time I have to import from a Task class in my specs I have to create a new instance in of it, what could become quite nasty when importing from more classes. Every time I write a new Task or PageElement class I have to ensure, that the translate object is initialised.

I tried to think my use case the Serenity/JS way. Isn’t it the Ability of an Actor to SpeakALanguage? Would it not be a an interaction to Learn.newVocabulary(<<object>>) and a Question to Translate.usingPlaceholder(‘SAVE’)

I started an experiment which already is functional, but I still have problems to use this in some locators, but I think I messed something up with return types, we can discuss this later.

Before I continue I’d like to get you opinion if I’m on the right path and if such a thing would be worth a contribution to Serenity/JS?

@viper3400
Copy link
Sponsor Collaborator Author

@jan-molak Not even sure, if you get informed by mail when just a draft for a PR is created.

@jan-molak
Copy link
Member

jan-molak commented Feb 19, 2022

Hey! Thanks for the ping, no, GitHub doesn't seem to send notifications for draft PRs

I tried to think my use case the Serenity/JS way. Isn’t it the Ability of an Actor to SpeakALanguage?
Would it not be a an interaction to Learn.newVocabulary(object) and a Question to Translate.usingPlaceholder('SAVE')

Yes, I'd approach it this way too. I like the idea of using an ability for that; it has to hold some state, so this sounds like a good way to do it.

For example, given our actor had an ability to Translate, for example:

actorCalled('Travis')
  .whoCan(Translate.using(dictionary))

Where En_Us is the language we will express our tests in, and Dictionary is something of this shape:

{
  'Save': {    
    En_Us: 'Save',
    De_Ch: 'Speichern',
    Fr_Ch: 'Enregistrer'
  }
}

I think injecting a dictionary might be easier than providing each translation individually, since this way it could be stored in a JSON file or sth like this.

Now, assuming that the website is in De_Ch and the tests are in En_Us, we could have a question, say Translation, that translates any string using that dictionary:

  Ensure.that(Translation.to('En_Us').of(Text.of(...)), equals('Save'))

which offers good flexibility since we don't need to care about the language the website is currently in.

Or do it on the expectation side too if we wanted:

  Ensure.that(Text.of(...), equals(Translation.to('De_Ch').of('Save')))

We could also have a way to specify a "default" language when defining the ability

actorCalled('Travis')
  .whoCan(Translate.using(dictionary, 'En_Us'))

Then it could be simplified further:

  Ensure.that(Translation.of(Text.of(...)), equals('Save'))
  Ensure.that(Text.of(...), equals(Translation.of('Speichern')))

Using of would also allow us to make Translation a MetaQuestion<Answerable<string>>, so it would work in both Web and API tests.

Let me know what you think, it seems like a really good feature to add to @serenity-js/core!

@viper3400
Copy link
Sponsor Collaborator Author

We could also have a way to specify a "default" language when defining the ability

Agreed.

I think injecting a dictionary might be easier than providing each translation individually, since this way it could be stored in a JSON file or sth like this.

Agreed. I assume my examples in my test specs are a bit misleading. The reason why I would like to use an array of dictionaries: I still have a external Serenity/JS library which servers shared Tasks, PageElements and a shared dictionary as well.

I have (for now) two separate test projects for different modules. Both module use the "outer" application framework and therefore make use of the common library - and the common translations. But each module has its very specific set of translations (or even each task or page elements class). So with an array of dictionaries I can inject more than just one object.

Also agreed, that it could be some JSON. I would even like to see some typed dictionary because it's easy to mess up with the translation strings, so an IDE support would be nice. But maybe your approach will mitigate this problem a bit.

Okay, so let's talk about the nature of the dictionary later. I got another point I struggle with.

So, given, we have an Ability called Translate:

import { Ability, AnswersQuestions, UsesAbilities } from '..';
import { LogicError } from '../../errors';
import { formatted } from '../../io';

export class Translate implements Ability {
    constructor(private readonly dictionaries: Array<unknown>, private readonly locale: string) { }

    private translations: unknown;

    static as(actor: UsesAbilities & AnswersQuestions): Translate {
        return actor.abilityTo(Translate);
    }

    static using(dictionaries: Array<unknown>, locale: string) {
        return new Translate(dictionaries, locale)
    }

    translate(translationString: string): Promise<string> {
        try {
            return this.translations[translationString][this.locale] ?? [translationString][this.translations['en_US']];
        } catch (error) {
            throw new LogicError(formatted`Translation error for "${translationString}" and locale "${this.locale}": ${error}`)
        }
    }
}

The pure logic - in my example the translate method - will still stay in the ability, right? Or would this be an Interaction?
Than I really struggle how to implement the MetaQuestion interface. How the of has to be implemented?

Sorry for still not having a complete picture of the framework.

@jan-molak
Copy link
Member

jan-molak commented Feb 19, 2022

Also agreed, that it could be some JSON. I would even like to see some typed dictionary because it's easy to mess up with the translation strings, so an IDE support would be nice. But maybe your approach will mitigate this problem a bit.

Thinking about it, rather than hand-rolling our own standard, perhaps we could/should consider some XLIFF-compatible "shape" for the dictionaries. Many (most?) translation tools are compatible with it, and there's an NPM xliff module that we could use to transform dictionaries to and from XLIFF.

I think often when the system under test is translated by a 3rd party, translations might be provided in this standard, so Serenity/JS could load a dictionary that the main system is using...

I'm not sure if that's a good idea or not yet, just thinking aloud really.

I'm not set on XLIFF either; it's something I've used in the past in a similar context. There might be other / more popular alternatives?

The pure logic - in my example the translate method - will still stay in the ability, right?

Yes, pretty much. An ability would handle the dictionaries and retrieve/lookup specific translations. The question would be just an adapter between abilities and anything that needs that data.

@viper3400 viper3400 force-pushed the feature/translation branch 2 times, most recently from 7ee27ff to cbf7846 Compare February 20, 2022 16:14
@viper3400 viper3400 marked this pull request as ready for review February 20, 2022 17:23
@viper3400
Copy link
Sponsor Collaborator Author

viper3400 commented Feb 20, 2022

Following your suggestions, I've created a draft of an Ability and a Question that implements MetaQuestion. I also enhanced the tests for this. Code is not documented, yet. Would appreciate if you could do a general review of it.

Don't know why SauceLabs Pipeline fails, probably some permission issues.

Thinking about it, rather than hand-rolling our own standard, perhaps we could/should consider some XLIFF-compatible "shape" for the dictionaries.

I was not aware that there is a standard for this. Before I started with my own translateTo function I looked into something like https://www.npmjs.com/package/i18next. I thought the same as you, why to create own things, instead of using existing stuff.

But than I found, that all those things would be over the top for my usage. As I separate some shared translations from the module specific translation and as I won't check every possible translation in the UI, my lists stay rather small.

Then I thought of my fellow testers, who would have to learn the next fancy syntax how to get things translated. This is where I appreciated the simple translateTo approach, you suggested me once.

I think often when the system under test is translated by a 3rd party, translations might be provided in this standard, so Serenity/JS could load a dictionary that the main system is using...

This was something I first thought about as well. Well, not exactly the same way, not the way with the XLIFF files. But why not use the string lists from the application (as most of the strings would/should be dynamic translations as well.

Again, this was a bit over the top, given the thoughts I shared before.

Last but not least, I tried to reduce npm dependencies instead of increasing them, lately ...

Maybe the current solution of the const translations object is not the best choice and can be optimized. But the solution is a quite simple one, obviously working well with just some lines of code.

But as Serenity/JS is becoming an "agnostic" tool, maybe there would be an extendible approach as well.

actor.whoCan(Translate.using('XLIFF').with(<<whatever would be necessary here>>));
// or
actor.whoCan(Translate.using('CustomFormat').with(<<whatever would be necessary here>>));

EDIT:

I also noticed I would need a Question that would take an array of strings to translate in order to compare arrays as well. I've called this Translations instead of Translation.

Then I added a question that returns the locale current locale of the Actors as I need this question to be answered in my tests.

@viper3400 viper3400 marked this pull request as draft February 20, 2022 17:27
@viper3400 viper3400 force-pushed the feature/translation branch 4 times, most recently from f47ffd2 to 2b5b9d9 Compare February 27, 2022 14:01
@viper3400
Copy link
Sponsor Collaborator Author

viper3400 commented Feb 27, 2022

Hi @jan-molak, what do you think about this (last comment) ?

In addition, I noticed some unexpected behaviour: How are the names of the actors connected to the actors I engage? Though I engage new actors before each step in the specs "Heidi" answers with 'de_CH' no matter if I put her in the UsAmerican or in the Swiss German spec? I thought this were just strings for the reporting and to be able to distinguish the actors in the test specs, but now it seems, as they where somehow tied to the Actor object?

Or do I have some problems in my Abilities to keep the right state?

https://github.com/viper3400/serenity-js/blob/2b5b9d995db8d41c3d02b48ebbc46021da98a395/packages/core/spec/screenplay/TranslationUSAmericanActor.spec.ts#L66

@jan-molak
Copy link
Member

jan-molak commented Feb 27, 2022

Don't know why SauceLabs Pipeline fails, probably some permission issues.

Looks like we need to limit running sauce tests to pull requests and branches originating from the main monorepo. Let me see if I can fix that.

But as Serenity/JS is becoming an "agnostic" tool, maybe there would be an extendible approach as well.

Yeah, that's a good point. You could see it work similarly to @serenity-js/web et al. 🤔
Then this PR becomes a standalone Serenity/JS plugin... 🤔 OK, before the scope creep creeps in, let's start small and see where we end up 😄

In addition, I noticed some unexpected behaviour: How are the names of the actors connected to the actors I engage? Though I engage new actors before each step in the specs "Heidi" answers with 'de_CH' no matter if I put her in the UsAmerican or in the Swiss German spec? I thought this were just strings for the reporting and to be able to distinguish the actors in the test specs, but now it seems, as they where somehow tied to the Actor object?

That's right, each Actor has a property name used to identify that actor and to ensure that calling actorCalled('Heidi') several times in the same test scenario returns the same instance of that actor. The first call to actorCalled('Heidi') instantiates and "caches" it, and subsequent call to actorCalled('Heidi') retrieves the instantiated actor.
Have a look at the Stage to see how this works internally.

The reason for this is so that people could write Cucumber scenarios like this:

Given Heidi ...
 When Heidi ...
 Then Heidi ...

where each Cucumber step references the same actor by their name.

Does this make sense?


Also, I wonder if translationDefaultLocale should become Translation.defaultLocale().
Making it a factory method on Translation could make it easier for people to find.

jan-molak added a commit that referenced this pull request Mar 2, 2022
because they won't have access to saucelabs username and access key and will therefore always fail

re #1135
@jan-molak
Copy link
Member

Don't know why SauceLabs Pipeline fails, probably some permission issues.

I think it should be fixed now on main. Could you rebase, please?

@viper3400
Copy link
Sponsor Collaborator Author

I think it should be fixed now on main. Could you rebase, please?

Thanks, I will do this as soon as possible.

Making it a factory method on Translation could make it easier for people to find.

What do you have in mind? Something like that? Just provide a method in Translation to return the question?

grafik

@viper3400
Copy link
Sponsor Collaborator Author

I think it should be fixed now on main. Could you rebase, please?

Fixed.

@viper3400 viper3400 marked this pull request as ready for review March 5, 2022 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants